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

Add ImprintPage model #357

Closed
timobrembeck opened this issue Apr 17, 2020 · 4 comments · Fixed by #488
Closed

Add ImprintPage model #357

timobrembeck opened this issue Apr 17, 2020 · 4 comments · Fixed by #488
Assignees
Labels
feature New feature or request good first issue Good for newcomers prio: medium Should be scheduled in the forseeable future.

Comments

@timobrembeck
Copy link
Member

timobrembeck commented Apr 17, 2020

Add an Imprint and ImprintTranslation model which should be similar to Page and PageTranslation.

@timobrembeck timobrembeck added feature New feature or request good first issue Good for newcomers prio: medium Should be scheduled in the forseeable future. labels Apr 17, 2020
@timobrembeck timobrembeck added this to the Feature Completion milestone Apr 17, 2020
This was referenced Apr 17, 2020
@Thomas125
Copy link
Contributor

I made a test by creating to test-models one "Oberklasse" and one "Unterklasse" where Unterklasse inherits from Oberklasse.
When I query all Oberklasse-instance I get all instances, no matter which class was chosen on creation.
(When I query all Unterklasse-instances I only get the instances which were explicity created as Unterklasse-Instances, of course)

So, to keep the behaviour that "Page.objects.all()" only returns the "content pages" (not the disclaimer page) we would need something like a "GeneralPage" which is inherited from Page. Analog thing with PageTranslation.

Under these circumstances I find it more sensible to create a DisclaimerTranslation-model with the attributes region, language and text with region and language as composite primary key (or as constraint unique_together).
To handle the feedback, we can add a flag is_desclaimer_feedback to the region-feedback-model, which defaults to False.

@Thomas125
Copy link
Contributor

Thomas125 commented Apr 17, 2020

Does the disclaimer page and its translations have the same translation workflow like the normal pages? Do we also keep older versions?
So do we need the attributes currently_in_translation, version and minor_edit for a disclaimer page?

@timobrembeck
Copy link
Member Author

timobrembeck commented Apr 19, 2020

Oh sorry, it seems that I should have done better research before proposing this.

I found a few ways how to achieve the desired behaviour with a custom model manager, but this is kind of hacky and not worth the drawbacks I think.

So yes, probably it's best to define these as standalone classes... if we use an abstract base class for the content in the future, we can make our optimizations there.

And yes, I'd propose to use the same versioning and translation system as for pages, including all special fields.

@timobrembeck
Copy link
Member Author

timobrembeck commented Jul 27, 2020

One other option would be to define an AbstractBasePage which is the parent model of Page and Imprint and an AbstractBasePageTranslation which is the parent model of PageTranslation and ImprintTranslation.
Maybe, this can also be implemented together with #320 Refactor translation models.

@timobrembeck timobrembeck changed the title Add DisclaimerPage model Add ImprintPage model Jul 28, 2020
@steffenkleinle steffenkleinle self-assigned this Jul 28, 2020
@steffenkleinle steffenkleinle linked a pull request Sep 18, 2020 that will close this issue
timobrembeck pushed a commit that referenced this issue Sep 24, 2020
- Add AbstractBasePage for attributes and methods shared with other page types
- Keep tree-related attributes and methods in the page model

(Prerequisite for #357)
timobrembeck pushed a commit that referenced this issue Sep 24, 2020
- Add AbstractBasePageTranslation for attributes and methods shared with other page translation types
- Keep tree- and revision-related attributes and methods in the page translation model

(Prerequisite for #357)
timobrembeck pushed a commit that referenced this issue Sep 24, 2020
- Add AbstractBasePage for attributes and methods shared with other page types
- Keep tree-related attributes and methods in the page model

(Prerequisite for #357)
timobrembeck pushed a commit that referenced this issue Sep 24, 2020
- Add AbstractBasePageTranslation for attributes and methods shared with other page translation types
- Keep tree- and revision-related attributes and methods in the page translation model

(Prerequisite for #357)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good first issue Good for newcomers prio: medium Should be scheduled in the forseeable future.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants