Skip to content

Don't nil publication info if article is already published - #5448

Merged
just1602 merged 1 commit into
crimethinc:push_kptxxmkxmqrlfrom
Bargraph6:push_kptxxmkxmqrl
Jul 1, 2026
Merged

just1602 merged 1 commit into
crimethinc:push_kptxxmkxmqrlfrom
Bargraph6:push_kptxxmkxmqrl

Conversation

@Bargraph6

@Bargraph6 Bargraph6 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

What does this pull request do?

  • app/controllers/admin/articles_controller.rb (Admin::ArticlesController): updated the permitted_params logic for articles to delete nil publication date info if the article is already published.
  • spec/requests/article_datetime_settings_spec.rb: Updated spec to allow for separate expected values for the update and create controller actions.

Is there any background context you want to provide for reviewers?

I don't think this is a situation that ever really happens in practice, however while updating the article_datetime_settings_spec, it was discovered that it was technically possible to pass nil to the update action of the articles controller.

This would result in an error further down the line as Article#path would try to strftime the nil:

     NoMethodError:
       undefined method 'strftime' for nil
     Shared Example Group: "handles the publication date" called from ./spec/requests/article_datetime_settings_spec.rb:61
     # ./app/models/article.rb:56:in '
     # ./app/models/article.rb:150:in 'Article#update_or_create_redirect'
     # ./app/controllers/admin/articles_controller.rb:56:in 'Admin::ArticlesController#update'
     # ...

This change just updates the article_params method to remove the published_at and published_at_tz params from the permitted params if the article is already published and the params are nil.


related_to: #5391

* app/controllers/admin/articles_controller.rb (Admin::ArticlesController): updated the permitted_params logic for articles to delete nil publication date info if the article is already published.
* spec/requests/article_datetime_settings_spec.rb: Updated spec to allow for separate expected values for the update and create controller actions.

I don't think this is a situation that ever really happens in
practice, however while updating the `article_datetime_settings_spec`,
it was discovered that it was technically possible to pass `nil` to
the update action of the articles controller.

This would result in an error further down the line as `Article#path`
would try to `strftime` the `nil`:

```sh
     NoMethodError:
       undefined method 'strftime' for nil
     Shared Example Group: "handles the publication date" called from ./spec/requests/article_datetime_settings_spec.rb:61
     # ./app/models/article.rb:56:in '
     # ./app/models/article.rb:150:in 'Article#update_or_create_redirect'
     # ./app/controllers/admin/articles_controller.rb:56:in 'Admin::ArticlesController#update'
     # ...
```

This change just updates the `article_params` method to remove the
`published_at` and `published_at_tz` params from the permitted params
if the article is already published and the params are `nil`.
@Bargraph6
Bargraph6 force-pushed the push_kptxxmkxmqrl branch from b0d023f to ac93659 Compare June 29, 2026 09:48
@just1602
just1602 merged commit b3bbc60 into crimethinc:push_kptxxmkxmqrl Jul 1, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants