[18.0][FIX] document_page: Don't change last history name and summary…

[diggy128]

… when

creating new revision of page.

Fixes an issue where editing a document.page and changing name and/or summary (to create a new revision) also changes name and summary of the history_head.

draft_name and draft_summary are now computed fields (based on history_head) but have set the inverse without actually setting the value, to allow setting them when creating a new draft page.

The issue is prominently evident when using document_page_approval where the name/summary for the new draft also change the last approved version (aka history_head) which should not happen.

[etobella]

Technically makes sense

[pedrobaeza]

But with this, you cannot change the head name and summary...

[etobella]

IT is not working....

If you write only one of the fields, the other are resetted...

[diggy128]

But with this, you cannot change the head name and summary...

In fact you can, because we set inverse on the field (even though not doing anything about it in the inverse method).

If there is a history_head, then these are fetched along with content, otherwise they are kept empty.

This way we always have the correct name and summary from history head, but allows to change them if we change the content

[diggy128]

IT is not working....

If you write only one of the fields, the other are resetted...

That is how I made it work.
When creating a new "version" of a document we should always set a new name and summary describing the changes.
So keeping the same name or summary when the content has changed is wrong.
If I've mistaken something about the usage of these fields let me know and I'll adapt the code.

However, especially in ISO certifications related documentation, having a document history with the same version and / or summary is wrong.
In our internal system, name is used just to increment a version number and summary to describe the changes made.
At some point we could add an analog to Semantic versions to history documents.

[etobella]

Yes, I know, however, this might give errors latelly. The don't changed are lost, and that is not an option.

[diggy128]

Sorry, can't quite understand what you mean. Can you please explain further?

[diggy128]

Sorry, can't quite understand what you mean. Can you please explain further?

@etobella Can you please elaborate?

[etobella]

@diggy128 there is an issue when you only edit one of the fields. I know that users shouldn't do it this way, but they can, so we cannot allow failires on the software and say: it is your fault because you didn't follow the expected workflow when the syatem never raises a warning.

also, i can imagine cases where the user makes a change without changing the fields for a real reason (like fixing 2 typo errors in a row (message could be the same))

[diggy128]

I am baffled. I don't seem to get if the module works as it was designed or there are things that should be fixed.

Throughout the years, there were a couple of things I found that didn't work as they were supposed to work:

1. History name and summary are displayed in document.page - I think they belong in the history documents. They should only be used when saving changes to describe the changes made and NOT be pulled from the last history document
2. When creating a new version of the document, name and summary are copied over from the previous version - I think this is a bug from the way it is implemented
3. Changing the name and / or summary when creating a new version (when the document requires approval) also changes the last approved document's name / summary - Approved / historical history documents should not be altered in any way!
4. Changing a document that requires approval creates a new history document that is automatically sent for approval - I think the document should stay in draft until the author deems it ready for review and approval (an email is also automatically sent to the approving user so the option to reset to draft and edit is not viable)
5. DB id used as a revision number - I think id is an internal implementation detail and should not be exposed to the user. Also the numbers are not sequential per document so a history document can have id 152 and the next one 210 (no clear versioning path). Revision numbering should be unique and sequential per document.
6. For each category / template a revision table should the contents - Refer to the following image

@etobella @pedrobaeza
Can I get your opinion for the points above?
If you think they should be changes, I'll be happy to propose the changes.
If you think the module works fine as is (and was designed), I'll put my changes to an internal module instead.

[diggy128]

@etobella @pedrobaeza
If you find some time, your input is valuable as I want to close this issue either way.

[etobella]

/ocabot rebase

[OCA-git-bot]

Congratulations, PR rebased to 18.0.

[etobella]

@diggy128 I understand your problems, and that makes me thing if saving document should open a popup to add this extra information... however, as I suggested before, your approach is not working as expected.

[diggy128]

@etobella that makes me thing if saving document should open a popup to add this extra information...

In this case we should cater for minor changes (eg typos) carrying over the summary.

@etobella however, as I suggested before, your approach is not working as expected.

Is the expected behavior the intended behavior?
I mean:

1. You WANT name and summary to be copied over from the last version?
2. When changing the name and / or summary you WANT to also change the last approved version along with the new draft?

[etobella]

@diggy128 I don't know if this is the expected behavior, however, leaving it empty when it should be required is obviously not a better outcome.

With the current situation, the current solution seems better than your proposal. Sorry if I am too direct, however, leaving space for mistakes of the users seems a worse outcome. We cannot expect that users know that they need to do everything at the same time to receive the expected outcome. That might not happen, so we need to propose something that works for them in all possible scenarios, not only on the one where everything works perfect.

Even if they make it as expected, users might change the version later on because they decided to make it a major change, not a minor change, also, the summary must be kept.

Personally, I think we should rethink the whole document page module to fit all the possible options, but that would require a refactoring of the module.