Skip to content

Commit

Permalink
Finished migrations, removed deprecated WorkStatus from exports
Browse files Browse the repository at this point in the history
  • Loading branch information
brendan-oconnell committed May 28, 2024
1 parent c4dfd37 commit d8d9983
Show file tree
Hide file tree
Showing 15 changed files with 87 additions and 172 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Changed
- [595](https://github.com/thoth-pub/thoth/issues/595) - Remove infrequently and unused work statuses, require a publication date for active works in Thoth

### Added
- [595](https://github.com/thoth-pub/thoth/issues/595) - Add a new `Superseded` work status

## [[0.12.5]](https://github.com/thoth-pub/thoth/releases/tag/v0.12.5) - 2024-05-07
### Changed
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@


-- Drop constraints, otherwise it won't be able to cast to text
ALTER TABLE work
DROP CONSTRAINT work_active_withdrawn_date_check,
DROP CONSTRAINT work_inactive_no_withdrawn_date_check;
-- TODO: drop new constraints in up migration
-- Drop constraints originally from v0.12.3,
-- otherwise it won't be able to cast to text
DROP CONSTRAINT IF EXISTS work_inactive_no_withdrawn_date_check,
DROP CONSTRAINT IF EXISTS work_active_withdrawn_date_check,
-- Drop new constraint from v.0.12.6
DROP CONSTRAINT IF EXISTS work_active_publication_date_check;

ALTER TABLE work ALTER COLUMN work_status TYPE text;

-- !!! if this down migration is run, 'out-of-print' should
-- be treated as a placeholder work_status.
-- Works will need to be manually reassigned correct work_status.
-- Works will need to be manually reassigned correct work_status:
-- out-of-print, out-of-stock-indefinitely, or inactive
-- This needs to be run because superseded is a new work_status
-- that is removed in this down migration.
UPDATE work
SET work_status = 'out-of-print'
WHERE work_status = 'superseded';

DROP TYPE work_status;

CREATE TYPE work_status AS ENUM (
'unspecified',
'cancelled',
Expand All @@ -31,6 +35,7 @@ CREATE TYPE work_status AS ENUM (
'withdrawn-from-sale',
'recalled'
);

ALTER TABLE work ALTER COLUMN work_status TYPE work_status USING work_status::work_status;

-- add constraints back to work table
Expand All @@ -41,4 +46,5 @@ ALTER TABLE work

ADD CONSTRAINT work_inactive_no_withdrawn_date_check CHECK
(((work_status = 'withdrawn-from-sale' OR work_status = 'out-of-print') AND withdrawn_date IS NOT NULL)
OR (work_status NOT IN ('withdrawn-from-sale', 'out-of-print')));
OR (work_status NOT IN ('withdrawn-from-sale', 'out-of-print')));

Original file line number Diff line number Diff line change
@@ -1,56 +1,67 @@
-- Assign updated_at as placeholder publication_date for Active works with no publication date
-- works in local db with this status as of 17-05-2024: 1 work
-- Assign 1900-01-01 as placeholder publication_date for
-- Active works with no publication date
-- works in local db with this status as of 17-05-2024: 51 works (incl. chapters)
-- Publisher should be notified and add correct publication_date
-- !!! This step of migration is irreversible.
UPDATE work
SET
publication_date = COALESCE(publication_date, updated_at)
publication_date = '1900-01-01'
WHERE
work_status = 'active';
work_status = 'active' AND publication_date IS NULL;

-- Drop constraints, otherwise it won't be able to cast to text
ALTER TABLE work
DROP CONSTRAINT work_active_withdrawn_date_check,
DROP CONSTRAINT work_inactive_no_withdrawn_date_check;
DROP CONSTRAINT IF EXISTS work_active_withdrawn_date_check,
DROP CONSTRAINT IF EXISTS work_inactive_no_withdrawn_date_check;

ALTER TABLE work ALTER COLUMN work_status TYPE text;

-- delete unused work_status enum
DROP TYPE work_status;

-- assign works new work_status if theirs is going to be deleted
-- current counts in production db as of 17-05-2024:
-- out of print, 17 works
-- inactive, 9 works
-- out of stock indefinitely, 0 works
-- !!! superseded is a placeholder for these works.
-- current counts for out of print/inactive/out of stock indefinitely in local db as of 17-05-2024:
-- 148 works (incl. chapters)
-- superseded seems to be the best placeholder for these works,
-- since it's a new status and no works are currently using it.
-- works will need to be manually reassigned correct work_status from the new
-- options, and withdrawn_date removed as necessary. This step in the migration is irreversible.
-- options, and withdrawn_date removed as necessary.
-- !!! This step in the migration is irreversible.
UPDATE work
SET
work_status = 'superseded',
withdrawn_date = COALESCE(withdrawn_date, updated_at)
-- assign a withdrawn_date, which will be required for superseded works
withdrawn_date = CASE
WHEN withdrawn_date IS NOT NULL THEN withdrawn_date
-- + INTERVAL '1 day' is necessary because one work has publication_date on
-- the same day as updated_at, but updated_at has a timestamp, so it's
-- greater than. Which then throws an error with the
-- work_withdrawn_date_after_publication_date_check constraint.
WHEN withdrawn_date IS NULL AND publication_date + INTERVAL '1 day' < updated_at THEN updated_at
ELSE CURRENT_DATE
END
WHERE
work_status = 'out-of-print'
OR work_status = 'out-of-stock-indefinitely'
OR work_status = 'inactive';



-- these work_status currently have no works with this status in production db;
-- nonetheless, reassign in case works get assigned these work_status
-- before migration is run in production
-- current counts in production db as of 17-05-2024:
-- current counts in local db as of 27-05-2024:
-- unspecified, 0 works
-- unknown, 0 works
-- !!! This step of the migration is irreversible, because of merging two
-- different work_status into a single one, and there are lots of
-- real works with forthcoming work_status that we don't want to
-- incorrectly change to 'unspecified' or 'unknown'. However, this doesn't
-- incorrectly change back to 'unspecified' or 'unknown'. However, this doesn't
-- seem like a big deal, since there are no works with these work_status
-- currently.
UPDATE work
SET work_status = 'forthcoming'
WHERE work_status = 'unspecified' OR work_status = 'unknown';

-- current counts in production db as of 17-05-2024:
-- current counts in local db as of 27-05-2024:
-- no-longer-our-product, 0 works
-- remaindered, 0 works
-- recalled, 0 works
Expand All @@ -64,30 +75,32 @@ UPDATE work
OR work_status = 'remaindered'
OR work_status = 'recalled';

-- delete unused work_status enum
DROP TYPE work_status;
-- add new values by creating new enum and add superseded

-- create new work_status enum, adds superseded
CREATE TYPE work_status AS ENUM (
'cancelled',
'forthcoming',
'postponed-indefinitely',
'active',
'withdrawn-from-sale',
'superseded',
'superseded'
);
ALTER TABLE work ALTER COLUMN work_status TYPE work_status USING work_status::work_status;

-- add new constraints (with same names as in v0.12.3) to work table
ALTER TABLE work
-- withdrawn and superseded works must have withdrawn_date
-- note that this constraint has the same name as migration from v.0.12.3,
-- but changes that constraint by adding superseded alongside withdrawn-from-sale
ADD CONSTRAINT work_inactive_no_withdrawn_date_check CHECK
(((work_status = 'withdrawn-from-sale' OR work_status = 'superseded') AND withdrawn_date IS NOT NULL)
OR (work_status NOT IN ('withdrawn-from-sale', 'superseded')));
-- all other work statuses must not have withdrawn_date
OR (work_status NOT IN ('withdrawn-from-sale', 'superseded'))),
-- all other work statuses must not have withdrawn_date; see above, adds superseded
ADD CONSTRAINT work_active_withdrawn_date_check CHECK
((work_status = 'withdrawn-from-sale' OR work_status = 'superseded')
OR (work_status NOT IN ('withdrawn-from-sale', 'superseded') AND withdrawn_date IS NULL)),
-- active works must have publication_date
ADD CONSTRAINT work_active_publication_date_check CHECK
((work_status = 'active' AND publication_date IS NOT NULL)
OR (work_status != 'active'))
OR (work_status != 'active'));

74 changes: 12 additions & 62 deletions thoth-api/src/model/work/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,25 +54,15 @@ pub enum WorkType {
#[serde(rename_all = "SCREAMING_SNAKE_CASE")]
#[strum(serialize_all = "title_case")]
pub enum WorkStatus {
Unspecified,
Cancelled,
#[default]
Forthcoming,
#[cfg_attr(feature = "backend", db_rename = "postponed-indefinitely")]
PostponedIndefinitely,
Active,
#[cfg_attr(feature = "backend", db_rename = "no-longer-our-product")]
NoLongerOurProduct,
#[cfg_attr(feature = "backend", db_rename = "out-of-stock-indefinitely")]
OutOfStockIndefinitely,
#[cfg_attr(feature = "backend", db_rename = "out-of-print")]
OutOfPrint,
#[default]
Inactive,
Unknown,
Remaindered,
#[cfg_attr(feature = "backend", db_rename = "withdrawn-from-sale")]
WithdrawnFromSale,
Recalled,
Superseded,
}

#[cfg_attr(
Expand Down Expand Up @@ -333,8 +323,8 @@ pub struct WorkOrderBy {
}

impl WorkStatus {
fn is_withdrawn_out_of_print(&self) -> bool {
matches!(self, WorkStatus::OutOfPrint | WorkStatus::WithdrawnFromSale)
fn is_withdrawn(&self) -> bool {
matches!(self, WorkStatus::WithdrawnFromSale)
}
}

Expand All @@ -343,23 +333,23 @@ pub trait WorkProperties {
fn publication_date(&self) -> &Option<NaiveDate>;
fn withdrawn_date(&self) -> &Option<NaiveDate>;

fn is_withdrawn_out_of_print(&self) -> bool {
self.work_status().is_withdrawn_out_of_print()
fn is_withdrawn(&self) -> bool {
self.work_status().is_withdrawn()
}

fn has_withdrawn_date(&self) -> bool {
self.withdrawn_date().is_some()
}

fn withdrawn_date_error(&self) -> ThothResult<()> {
if !self.is_withdrawn_out_of_print() && self.has_withdrawn_date() {
if !self.is_withdrawn() && self.has_withdrawn_date() {
return Err(ThothError::WithdrawnDateError);
}
Ok(())
}

fn no_withdrawn_date_error(&self) -> ThothResult<()> {
if self.is_withdrawn_out_of_print() && !self.has_withdrawn_date() {
if self.is_withdrawn() && !self.has_withdrawn_date() {
return Err(ThothError::NoWithdrawnDateError);
}
Ok(())
Expand Down Expand Up @@ -515,7 +505,7 @@ fn test_worktype_default() {
#[test]
fn test_workstatus_default() {
let workstatus: WorkStatus = Default::default();
assert_eq!(workstatus, WorkStatus::Inactive);
assert_eq!(workstatus, WorkStatus::Forthcoming);
}

#[test]
Expand Down Expand Up @@ -543,23 +533,11 @@ fn test_workstatus_display() {
"Postponed Indefinitely"
);
assert_eq!(format!("{}", WorkStatus::Active), "Active");
assert_eq!(
format!("{}", WorkStatus::NoLongerOurProduct),
"No Longer Our Product"
);
assert_eq!(
format!("{}", WorkStatus::OutOfStockIndefinitely),
"Out Of Stock Indefinitely"
);
assert_eq!(format!("{}", WorkStatus::OutOfPrint), "Out Of Print");
assert_eq!(format!("{}", WorkStatus::Inactive), "Inactive");
assert_eq!(format!("{}", WorkStatus::Unknown), "Unknown");
assert_eq!(format!("{}", WorkStatus::Remaindered), "Remaindered");
assert_eq!(
format!("{}", WorkStatus::WithdrawnFromSale),
"Withdrawn From Sale"
);
assert_eq!(format!("{}", WorkStatus::Recalled), "Recalled");
assert_eq!(format!("{}", WorkStatus::Superseded), "Superseded");
}

#[test]
Expand Down Expand Up @@ -637,10 +615,6 @@ fn test_worktype_fromstr() {
#[test]
fn test_workstatus_fromstr() {
use std::str::FromStr;
assert_eq!(
WorkStatus::from_str("Unspecified").unwrap(),
WorkStatus::Unspecified
);
assert_eq!(
WorkStatus::from_str("Cancelled").unwrap(),
WorkStatus::Cancelled
Expand All @@ -654,37 +628,13 @@ fn test_workstatus_fromstr() {
WorkStatus::PostponedIndefinitely
);
assert_eq!(WorkStatus::from_str("Active").unwrap(), WorkStatus::Active);
assert_eq!(
WorkStatus::from_str("No Longer Our Product").unwrap(),
WorkStatus::NoLongerOurProduct
);
assert_eq!(
WorkStatus::from_str("Out Of Stock Indefinitely").unwrap(),
WorkStatus::OutOfStockIndefinitely
);
assert_eq!(
WorkStatus::from_str("Out Of Print").unwrap(),
WorkStatus::OutOfPrint
);
assert_eq!(
WorkStatus::from_str("Inactive").unwrap(),
WorkStatus::Inactive
);
assert_eq!(
WorkStatus::from_str("Unknown").unwrap(),
WorkStatus::Unknown
);
assert_eq!(
WorkStatus::from_str("Remaindered").unwrap(),
WorkStatus::Remaindered
);
assert_eq!(
WorkStatus::from_str("Withdrawn From Sale").unwrap(),
WorkStatus::WithdrawnFromSale
);
assert_eq!(
WorkStatus::from_str("Recalled").unwrap(),
WorkStatus::Recalled
WorkStatus::from_str("Superseded").unwrap(),
WorkStatus::Superseded
);

assert!(WorkStatus::from_str("Published").is_err());
Expand Down
8 changes: 3 additions & 5 deletions thoth-app/src/component/new_work.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ impl Component for NewWorkComponent {
self.work.page_interval = None;
}
if self.work.work_status != WorkStatus::WithdrawnFromSale
&& self.work.work_status != WorkStatus::OutOfPrint
{
self.work.withdrawn_date = None;
}
Expand Down Expand Up @@ -458,9 +457,8 @@ impl Component for NewWorkComponent {
// Grey out chapter-specific or "book"-specific fields
// based on currently selected work type.
let is_chapter = self.work.work_type == WorkType::BookChapter;
let is_not_withdrawn_or_out_of_print = self.work.work_status
!= WorkStatus::WithdrawnFromSale
&& self.work.work_status != WorkStatus::OutOfPrint;
let is_not_withdrawn = self.work.work_status
!= WorkStatus::WithdrawnFromSale;
html! {
<>
<nav class="level">
Expand Down Expand Up @@ -533,7 +531,7 @@ impl Component for NewWorkComponent {
value={ self.work.withdrawn_date.clone() }
oninput={ ctx.link().callback(|e: InputEvent| Msg::ChangeWithdrawnDate(e.to_value())) }
required = true
deactivated={ is_not_withdrawn_or_out_of_print }
deactivated={ is_not_withdrawn }
/>
<FormTextInput
label = "Place of Publication"
Expand Down
8 changes: 3 additions & 5 deletions thoth-app/src/component/work.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,6 @@ impl Component for WorkComponent {
self.work.page_interval = None;
}
if self.work.work_status != WorkStatus::WithdrawnFromSale
&& self.work.work_status != WorkStatus::OutOfPrint
{
self.work.withdrawn_date = None;
}
Expand Down Expand Up @@ -578,9 +577,8 @@ impl Component for WorkComponent {
// Grey out chapter-specific or "book"-specific fields
// based on currently selected work type.
let is_chapter = self.work.work_type == WorkType::BookChapter;
let is_not_withdrawn_or_out_of_print = self.work.work_status
!= WorkStatus::WithdrawnFromSale
&& self.work.work_status != WorkStatus::OutOfPrint;
let is_not_withdrawn = self.work.work_status
!= WorkStatus::WithdrawnFromSale;
html! {
<>
<nav class="level">
Expand Down Expand Up @@ -661,7 +659,7 @@ impl Component for WorkComponent {
value={ self.work.withdrawn_date.clone() }
oninput={ ctx.link().callback(|e: InputEvent| Msg::ChangeWithdrawnDate(e.to_value())) }
required = true
deactivated={ is_not_withdrawn_or_out_of_print }
deactivated={ is_not_withdrawn }
/>
<FormTextInput
label = "Place of Publication"
Expand Down
Loading

0 comments on commit d8d9983

Please sign in to comment.