search

coderedcorp / coderedcms

Wagtail + CodeRed Extensions enabling rapid development of marketing-focused websites.
https://www.coderedcorp.com/cms
Other
691 stars 134 forks source link

Reusable content revisions #571

Closed jayden-arrai closed 9 months ago

jayden-arrai commented 1 year ago

Added wagtails revisions to reusable content. Wagtail history for a reusable content becomes available and can be accessed from the edit form through the status icon/panel or history icon in the top right. The history of pages and other reusable content will display the contents of reusable content they use, and any changes made to them. When a reusable content is saved, a revision will be created for any pages or reusable content that use it, to provide a way to see these changes.

Documentation has been added, along with an image showing a diff of changes to a reusable content using the html type.

Tests have been added to cover 100% of the code additions. A test exists for a reusable content used on a page. And a test exists for a reusable content used by a reusable content which is then used on a page, to test that it works recursively.

jayden-arrai commented 1 year ago

I didn't realize the website required migration changes. I'll get those done.

jayden-arrai commented 1 year ago

The basics of this is taken from: https://docs.wagtail.org/en/v4.1.2/topics/snippets.html#saving-revisions-of-snippets

vsalvino commented 1 year ago

This will be a great feature, thanks for doing the work, with tests and docs! Just a heads up that it may take me a bit of time to fully review this.

Thought: ideally all of our snippets could benefit from having revisions, especially navbar and footer. However based on this PR it seems like a lot of work.

And a second, contradictory thought: the ultimate plan is to eventually make all our snippets abstract (#56 ). Based on your experience with adding the revision and comparison, would you see any problems with making the snippet abstract in the future?

jayden-arrai commented 1 year ago

Sorry for not responding earlier, I was on vacation last week.

Wagtail has an AbstractPage class which uses the RevisionMixin. So presumably there shouldn't be any issues with switching the snippets to be abstract, since wagtail already is that way. https://github.com/wagtail/wagtail/blob/351bffa304f16d806f5be78d6b8ea43b97e080d8/wagtail/models/__init__.py#L1047-L1066

The reason why I've only done this for the reusable content, is because we have a client that uses the reusable content, but only that. They have a custom footer and navigation, to make it easier for them to manage, so that is why I have only set this up for the reusable content, since it is the only one they need history for.

jayden-arrai commented 1 year ago

I went to implement these changes in the project that needed it, and I found a few issues related to existing reusable content objects that didn't have a revision yet. Added some fixes. And I synced and renamed my migration because of a new one in the latest dev changes.

jayden-arrai commented 1 year ago

While deploying, I found another place that needed some code. I made some changes in the process of adding that, so the diffing of reusable content displays a bit nicer. image or if there isn't any changes: image

The tests for this will be broken for a bit. I need to make one for the new situation and update the ones that exist related to the html changes. Plus the image I captured should get adjusted. But I wanted to get the code changes in, so you can see what was also needed, in case you start working on similar changes to other snippet types. The next commit will contain the changes.

jayden-arrai commented 1 year ago

I should mention. I'll get the tests fixed up as soon as I can. But I have something else that I need to work on prior to that. I should have the changes fixed up within the next month, but I'm not sure when.

jayden-arrai commented 1 year ago

I guess the tests should become a bit better. They don't test the label in the history view, so they didn't fail.

jayden-arrai commented 1 year ago

The failing lint test appears to be in code that I haven't touched. Why would it not produce the same error in the dev branch when linting?

jayden-arrai commented 9 months ago

I've got a change related to wagtail, and a number of changes related to objects that do not have revisions yet (found after deploying to our site that uses this). So, I'm going to close this pull request and make a new one as soon as I can with all these changes. It is easier than trying to deal with the conflicts, since I did already in a different branch.