PRX / cms.prx.org

CMS API for PRX
https://cms.prx.org
GNU Affero General Public License v3.0
4 stars 2 forks source link

No unpublish side effects #544

Closed cavis closed 4 years ago

cavis commented 4 years ago

For PRX/publish.prx.org#698.

This prevents 2 bad cases, when updating story.published_at to match the story.released_at.

  1. If published (published_at <= now) ... don't allow nulling out published_at.
  2. If published (published_at <= now) ... don't allow setting published_at > now.

Some fallout on the frontend (PRX/publish.prx.org#702). But I think we always want to prevent these 2 cases on the backend in any case. The only API path to unpublishing something is POST /api/v1/stories/1234/unpublish.

svevang commented 4 years ago

Looks good, just found one side effect:

Given a scheduled story,
Select "publish" from the status dropdown
Click the "Publish Now" button
See the released_at date bounded to todays date
Click 'edit' next Dropdate and set a date far in the future
Click the "Publish Now" button
See a 409 error message
Try to make an unrelated change, e.g. to the title
Click the "Publish Now" button
See a 409 error message
Reload the page and see the released_at date is still far in the future

At this point, it seems I'm caught in a loop. Digging into the localstorage, it seems that the publish/unpublish box is holding onto the released_at attribute of the Story. If I clear out the stories from the localstorage, then the publish released at box shows the released_at date bounded with the published date.

Furthermore, checking out the console, the released_at attribute is nil throughout this process.

cavis commented 4 years ago

You're onboard with the CMS logic in this PR then?

Yeah, that's not great. Let's not merge this yet then, and I'll try to clean that up in PRX/publish.prx.org#702, and we can do a concurrent deploy. (I do like CMS doing the "right thing" to prevent this).