Uwazi: Changing an entity's template will destroy it's connections

[txau]

Eventually also other metadata may be affected since users have reported the "Publish" state not being kept.

Steps:

1. Create a document or entity.
2. Connect it to another document or entity.
3. Edit the first document and change it's template.
4. Save.
5. The connection with the second entity is lost.

As a temporal workaround, for newly created entities set the template before configuring connections and publishing.

[fnocetti]

The main problem seems to be that the line 111 in the following snippet explicitly deletes the relationships for the entity when the template is changed:
https://github.com/huridocs/uwazi/blob/fe5e550dc3a770e0f9cb044ea0734bedaa1138e5/app/api/entities/entities.js#L104-L113

Removing it will still delete the metadata relationship (in the previous line) but prevent the actual connection to be lost.
The tests continue to pass and many other scenarios that I could test continued to work just as currently.
I still need to test complex scenarios and understand if others flows depend on this, but I could use a pair of experienced eyes :)

I'll keep you posted.

[txau]

@fnocetti this could be related to text references and table of contents. Previously Uwazi had a hard differentiation between documents and entities. Now they are the same and the only thing that triggers the document UI is having a "main file" added. So entities and documents can go from one to another by adding or deleting this "main file".

When a file is changed, the existing TOC entries and text references are not valid anymore (since the source text has changed) and they need to be deleted. So that code snippet you mentioned may be related to that previous data workflow?

[fnocetti]

@txau thanks for the input. What you are suggesting is that maybe that snippet is deleting the relationships because for some flows it might need to get rid of content-referencing relationships?

I will do some testing to understand those flows, but this code is only executed when a new template is assigned to an entity so, a priori, I don't see this could be related to file updates.

[RafaPolit]

This is weird. I see no reason (not even on Text References) where changing the template would require deleting all relationships. But, as @fnocetti points out, this has been EXPLICITLY implemented. It's not a side effect, it has intentionally been set up as an outcome of changing the template.

Can you please @fnocetti look into when was this line added and as part of what commit / PR / Feature or bug fixing?

This may give us a insight as to what exactly we were trying to achieve with that deletion.

[fnocetti]

@RafaPolit I tracked it yesterday. The change was introduced in #901.
The related issue #899 describes a scenario similar to what are we facing here but the solution may have been proved in the future not to meet all the requirements, because it is deleting all the relationships.

The unit test associated to that change is no longer present in the code base. I'm trying to track down the commit it was removed on to understand why that piece of code is still there.

On the while, I noted there's no way to differentiate a connection that was created "standalone" from a connection that was created from a metadata relationship. If I understood well (please correct me if not) we should do delete the connections that where created from a metadata relationship that was in the previous template (when changing the template), which seems not to be possible (and I'm guessing thats what #901 was trying to do).

[RafaPolit]

@fnocetti This precedes even our current HUB implementation.

As you mention, there is no distinction between a relationship that was created via metadata, or via "standalone", nor there should be, in my opinion.

Also, simply assuming that the user meant to delete the relationships (even if made via metadata) upon template change, is assuming too much.

Obviously, all this was design for a no-longer-existing Uwazi: relationships simply do not work like that any more.

That code should have been deleted when the tests for that were deleted.

These are the scenarios where TDD make better sense to me: first see some test red. If no test is red, you cannot do anything! Even if that is deleting tests. If you delete tests, you need to at least see one test red and change code afterwards.

So yes, bottom line, there is no longer need for this, and it is breaking a lot of connections that are wanted in the process. Please, prevent that from happening. If that is deleting that single line, great! But please, ensure a tests that proves that changing template does not delete connections (but ideally that it deletes the metadata relationship fields that no longer meet the criteria).

[fnocetti]

@mfacar found the PR in which the related unit test was removed: #1520
Particularly in https://github.com/huridocs/uwazi/commit/6454ff8b15c9d9df61fe6440a3ab4ca1c3e9741e

It looks like it is the one implementing hubs or, at least, a huge development around connections. So it makes a lot of sense that the problematic code was left there unintentionally.

We are providing a PR (mostly likely today) to fix this.

[txau]

@RafaPolit do we still keep the field values when we change templates in the metadata? not only relationships, but all values.

[RafaPolit]

@txau It’s a requirement for a lot of instances, so I sincerely hope we do. That is, we keep the metadata properties that are common to the previous and newly selected template. But we should double check.

[fnocetti]

@txau @RafaPolit we do. But it relies on the front end to send the new metadata when doing an entity update. That's actually why not longer matching metadata relationships are being removed, the front end is just not posting them to the backend because they're not in the new template.
In the other hand, we faced an scenario when working only with the api (we saw it through a unit test) in which changing the template does not remove the non matching metadata relationships because the sanitization on the backend doesn't remove the properties that are not longer needed.

We are implementing an improvement to the sanitization function to remove the properties that don't exist in the new template. You think it's worth doing?

[RafaPolit]

I'd like @daneryl or @konzz input on this.

Yes, I believe its useful. But not on a 'prior template', 'new template' scenario. A simple sanitizing procedure that just strips the metadata properties that doesn't belong to the template included in the request.

That is, we don't do a "this was the old template, this is the new" type of check, we just do a "the entity lists in the request that it belongs to template X, I fetch template X and check for correct properties and types".

Yes, I think it's worth it. But, I'd like more feedback from the others as well.

[fnocetti]

We are working in that simple sanitization you said: we are not looking at a previous template, just checking that the metadata of the entity is no more than the metadata allowed by the template.

Agree, let's wait for their input 👍

[fnocetti]

We tried improving the sanitization but there are too many things affected by that change.
@mfacar will revert the commits in the branch that implements the refactor and adjust PR for only the original connections problem.

I will create another issue for the problem on the sanitization and we can discuss there it's complexity and urgency.

[mfacar]

I have reverted to the point that resolves the connections problem. It is opened a draft PR #2941 for your revision @RafaPolit @txau

[RafaPolit]

Our new agreement for flagging PRs is:

• Draft: a work in progress not ready to be merged, usually no one does QA unless asked for
• Non-draft: PR ready for QA and then merge.

Are you asking for an intermediate review or is this ready to be merged?

Thanks.

[mfacar]

It's ready for merge since @fnocetti has created a new issue that covers the following criteria of test:

it deletes the metadata relationship fields that no longer meet the criteria

I've changed the flag to Non-draft.

[RafaPolit]

We are going for the long-term good solution here: changing templates never deletes connections, but sanitizes the metadata properly.

[fnocetti]

@mfacar and I, we are posting here the advance of the investigation:

• After preventing the erasing of the connections upon template change of an entity, all the problems are related to the regeneration of the relationship metadata fields.
• At first, it looks like most, if not all, of the problems can be solved by invoking the functions entities.updateMetadataFromRelationships or relationships.updateEntitiesMetadataByHub in the correct places to recalculate those fields. The latest is of special interest because one of the problems need this recalculation to be executed on an entity other than the one we are editing (that is, every entity that is in the other side of the relationship). As it might lead to poor performance, we might want to stick with the entities function and pre-calculate the entities that need updates in advance.
• A more complex problem is that of real-time preview of the new metadata state on the Uwazi UI after changing the template in the form. At the moment there's no way to know in the client what are the values of the relationship fields going to be after their recalculation, and that is an important problem to solve because:
  
  
  
  • The user might want to do any action over the new state of those fields.
  
  
  • Since we are sticking to REST, we might end up clearing up those fields because, naturally, the client will send them empty. We still need to review if that is the actual scenario.
  
  
• To accomplish that, we are considering three options:
  
  
  
  • [ ] Generating a "preview" API that calculates the new metadata fields for the currently-editing entity, without persisting the results.
  
  
  • [ ] Require the user to save the template change previous to allow them to do further changes on the relationship fields.
  
  
  • [ ] Refactor the metadataFromRelationship functions to separate the calculation and persistence logic and move the calculation logic to the shared modules. Then use that logic in the client. How should that calculation functions be used in the client is still under review. Just as a brainstorm idea, we are considering using it not to modify the entity store, but to transform the values of the edition form.
  
  

[txau]

what are the values of the relationship fields going to be after their recalculation

@fnocetti can you elaborate on this please? maybe provide an example.

[fnocetti]

@txau sure. Consider this scenario:

• There are templates A, B and C; and a relationship type "related".
• Template A has a relationship metadata property for type "related" and template "B"
• Consider two entities tC and tB, and a connection from tC to tB of type "related".

Now if you change the template of entity tC to A, the connection it has to tB verifies the conditions to populate the relationship metadata field on template A. But you are not shown that in the UI.

What's more, since the checkbox is unchecked in the UI, the connection will be erased upon submitting the changes.
So the problem we are referring to is: how do you calculate the value of that checkbox on the relationship metadata field on the client, previous to persisting the template change on the DB?