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

Pages revisions feature #139

Merged
merged 4 commits into from
Oct 22, 2024
Merged

Conversation

Engerrs
Copy link
Member

@Engerrs Engerrs commented Oct 14, 2024

Hi @amercader and @wardi
With this PR I want to add Revision feature to the Pages and Blog HTML content.
It is a quite straightforward feature that adds and ability to Restore previous Content state in case if something went wrong while saving current version.
By default it creates only 3 versions, while can be increased with ckanext.pages.revisions_limit config setting.

It shouldn't effect old pages that were created before the Revisions logic update and the first Revision for those Pages/Blogs can be create by re-saving the current version.

It required DB upgrade after pulling.

Please review, test this and let me know if any adjustments are needed.

Copy link
Member

@amercader amercader left a comment

Choose a reason for hiding this comment

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

This looks good @Engerrs , I just left some minor comments.

We do need a couple tests though :)
The ones in test_logic.py should be easy to adapt. Just to cover the restore action and the preview and restore endpoints

ckanext/pages/utils.py Outdated Show resolved Hide resolved
ckanext/pages/utils.py Outdated Show resolved Hide resolved
ckanext/pages/actions.py Outdated Show resolved Hide resolved
@Engerrs
Copy link
Member Author

Engerrs commented Oct 20, 2024

Hi @amercader , added the mentioned updates and wrote test cases.

As for the PR checks, it seems to be failing on 2.9 CKAN, but doesn't look like it is related to my changes.

@amercader amercader merged commit 78ddf92 into ckan:master Oct 22, 2024
3 of 4 checks passed
@amercader
Copy link
Member

Thanks @Engerrs After making the 2.9 tests run again there were actual failures caused by inconsistencies across CKAN versions but I fixed those separately (063f03c)

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.

3 participants