Skip to content

Commit

Permalink
Added constraint to db for publication date required for active, with…
Browse files Browse the repository at this point in the history
…drawn from sale, and superseded works, updated PublicationDateError, made Publication Date field required in View for Active Works
  • Loading branch information
brendan-oconnell committed May 29, 2024
1 parent a04a0b9 commit 3fae5a8
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 48 deletions.
63 changes: 30 additions & 33 deletions thoth-api/migrations/v0.12.6/up.sql
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
-- 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.
-- Assign 1900-01-01 as placeholder publication_date for
-- Active, withdrawn from sale, out of print, out of stock indefinitely works with no publication date
-- Required for work_active_publication_date_check constraint below
-- Affected works in production db with this status, 29-05-2024: 59 works (incl. chapters)
-- Before running migration, make a list of affected works
-- After running migration, publishers should be notified to add correct publication_date
-- !!! This is irreversible
UPDATE work
SET
publication_date = '1900-01-01'
WHERE
work_status = 'active' AND publication_date IS NULL;
work_status IN
('active', 'withdrawn-from-sale', 'out-of-print', 'out-of-stock-indefinitely', 'inactive')
AND publication_date IS NULL;

-- Drop constraints, otherwise it won't be able to cast to text
ALTER TABLE work
Expand All @@ -19,21 +23,21 @@ 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 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.
-- Assign out of print/inactive/out of stock indefinitely works work_status 'superseded'
-- current counts in production db as of 29-05-2024:
-- 145 works (incl. chapters)
-- Before running migration, make a list of affected works
-- After running migration, publishers should be notified to add correct work_status
-- and remove withdrawn_date as necessary. Many OBP "out of print" works are actually first editions
-- for which superseded is the correct new work_status.
-- !!! This is irreversible
UPDATE work
SET
work_status = 'superseded',
-- assign a withdrawn_date, which will be required for superseded works
-- assign a withdrawn_date, which is 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
-- + INTERVAL '1 day' is necessary because at least 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.
Expand All @@ -45,27 +49,21 @@ UPDATE work
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 local db as of 27-05-2024:
-- Assign unspecified/unkown works work_status 'forthcoming'
-- current counts in production db as of 29-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 back to 'unspecified' or 'unknown'. However, this doesn't
-- seem like a big deal, since there are no works with these work_status
-- currently.
-- !!! This is irreversible
UPDATE work
SET work_status = 'forthcoming'
WHERE work_status = 'unspecified' OR work_status = 'unknown';

-- current counts in local db as of 27-05-2024:
-- Assign no longer our product/remaindered/recalled works work_status 'withdrawn-from-sale'
-- current counts in production db as of 29-05-2024:
-- no-longer-our-product, 0 works
-- remaindered, 0 works
-- recalled, 0 works
-- !!! see above: this step of the migration is irreversible.
-- !!! This is irreversible
UPDATE work
SET
work_status = 'withdrawn-from-sale',
Expand All @@ -75,7 +73,6 @@ UPDATE work
OR work_status = 'remaindered'
OR work_status = 'recalled';


-- create new work_status enum, adds superseded
CREATE TYPE work_status AS ENUM (
'cancelled',
Expand All @@ -91,15 +88,15 @@ ALTER TABLE work ALTER COLUMN work_status TYPE work_status USING work_status::wo
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
-- but changes previous 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; 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
-- active, withdrawn-from-sale, and superseded 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'));
((work_status IN ('active', 'withdrawn-from-sale', 'superseded') AND publication_date IS NOT NULL)
OR (work_status NOT IN ('active', 'withdrawn-from-sale', 'superseded')));
24 changes: 13 additions & 11 deletions thoth-api/src/model/work/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,13 @@ pub struct WorkOrderBy {
}

impl WorkStatus {
fn is_withdrawn(&self) -> bool {
matches!(self, WorkStatus::WithdrawnFromSale)
fn is_withdrawn_superseded(&self) -> bool {
matches!(self, WorkStatus::WithdrawnFromSale |
WorkStatus::Superseded)
}
fn is_active(&self) -> bool {
matches!(self, WorkStatus::Active)
fn is_active_withdrawn_superseded(&self) -> bool {
matches!(self, WorkStatus::Active |
WorkStatus::Superseded | WorkStatus::WithdrawnFromSale)
}
}

Expand All @@ -336,12 +338,12 @@ pub trait WorkProperties {
fn publication_date(&self) -> &Option<NaiveDate>;
fn withdrawn_date(&self) -> &Option<NaiveDate>;

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

fn is_active(&self) -> bool {
self.work_status().is_active()
fn is_active_withdrawn_superseded(&self) -> bool {
self.work_status().is_active_withdrawn_superseded()
}

fn has_withdrawn_date(&self) -> bool {
Expand All @@ -353,21 +355,21 @@ pub trait WorkProperties {
}

fn active_no_publication_date_error(&self) -> ThothResult<()> {
if self.is_active() && self.has_publication_date() {
if self.is_active_withdrawn_superseded() && !self.has_publication_date() {
return Err(ThothError::PublicationDateError);
}
Ok(())
}

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

fn no_withdrawn_date_error(&self) -> ThothResult<()> {
if self.is_withdrawn() && !self.has_withdrawn_date() {
if self.is_withdrawn_superseded() && !self.has_withdrawn_date() {
return Err(ThothError::NoWithdrawnDateError);
}
Ok(())
Expand Down
3 changes: 2 additions & 1 deletion thoth-app/src/component/new_work.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,8 @@ impl Component for NewWorkComponent {
self.work.last_page = None;
self.work.page_interval = None;
}
if self.work.work_status != WorkStatus::WithdrawnFromSale
if self.work.work_status != WorkStatus::WithdrawnFromSale &&
self.work.work_status != WorkStatus::WithdrawnFromSale
{
self.work.withdrawn_date = None;
}
Expand Down
6 changes: 3 additions & 3 deletions thoth-errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,17 @@ pub enum ThothError {
"Price values must be greater than zero. To indicate an unpriced Publication, omit all Prices."
)]
PriceZeroError,
#[error("Publication Date is required for an Active Work.")]
#[error("Publication Date is required for Active, Withdrawn From Sale, and Superseded Works.")]
PublicationDateError,
#[error("{0}")]
RequestError(String),
#[error("{0}")]
GraphqlError(String),
#[error("Withdrawn Date must be later than Publication Date.")]
WithdrawnDateBeforePublicationDateError,
#[error("Withdrawn Date can only be added to an Out of Print or Withdrawn From Sale Work.")]
#[error("Withdrawn Date can only be added to a Superseded or Withdrawn From Sale Work.")]
WithdrawnDateError,
#[error("An Out of Print or Withdrawn From Sale Work must have a Withdrawn Date.")]
#[error("A Superseded or Withdrawn From Sale Work must have a Withdrawn Date.")]
NoWithdrawnDateError,
}

Expand Down

0 comments on commit 3fae5a8

Please sign in to comment.