Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update published_at only after page has been published. #2331

Merged
merged 1 commit into from May 17, 2022
Merged

Update published_at only after page has been published. #2331

merged 1 commit into from May 17, 2022

Conversation

pascalbetz
Copy link
Contributor

@pascalbetz pascalbetz commented May 5, 2022

Updating published_at only after a public PageVersion has been created or updated.

What is this pull request for?

Issue #2339

Notable changes (remove if none)

Page#published_at is updated async.

Attribute is used in cache key and should only be updated once the page has been published. Since publication is now done in background, the timestamp needs to be updated in the background as well.

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

@pascalbetz pascalbetz changed the title Fix #2330 Update published_at only after page has been published. May 5, 2022
@tvdeyen
Copy link
Member

tvdeyen commented May 5, 2022

Thanks, there are some git conflicts and it would be great to have the two commits to be squashed into one. And if you are rebasing anyway, could you please move the issue reference into the commit message body? Thanks again

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are related spec fails. Can you take a look?

spec/models/alchemy/page_spec.rb Outdated Show resolved Hide resolved
@pascalbetz
Copy link
Contributor Author

Thanks for the feedback. I'll look into it.

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow a commit sneaked into here.


def ransackable_scopes(_auth_object)
[:published, :from_current_site, :searchables, :layoutpages]
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit does not belong here. Can you please rebase the branch and remove it?

Closes  #2330

Updating published_at only after a public PageVersion has been created or updated
to avoid e-tags being created with old PageVersion.

re-add Page#publish! since it was public interface
@pascalbetz
Copy link
Contributor Author

okay, done.
did not expect you to look into it so quickly and only worked in between on it :-)

@tvdeyen
Copy link
Member

tvdeyen commented May 6, 2022 via email

@pascalbetz
Copy link
Contributor Author

Hey Thomas

rebased the PR.

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@tvdeyen tvdeyen merged commit e65463e into AlchemyCMS:main May 17, 2022
tvdeyen added a commit that referenced this pull request May 30, 2022
Update published_at only after page has been published.
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.

None yet

2 participants