Update view Timestamp logic to fix setAmendmentState problem #4412
Conversation
There was a problem hiding this comment.
The motion-change-recommendation-controller.service.ts there is a method getChangeRecosOfMotionObservable that has the exact same code. If this code is broken, that one should be as well. Please check that.
If the change recommendation code is also broken, the entire viewModelUpdateTimestamp logic can be removed. If not, Maybe reconsider whether this distinctUntilChanged is actually at fault
|
I have tested the change recommendation handling with the more then one crs. It worked fine. |
|
I worked on fixing the update-timestamp at update, used the changedModels method for it. Test works. |
|
@luisa-beerboom Please look at the my new solution. |
reiterl
changed the title
There was a problem hiding this comment.
This still doen't fix the Issue:
I created a motion with 8 paragraphs
and added 5 amendments (in paragraph 1,2,3,7 and 8)
- Changing the first state sometimes works
- Sometimes it updates after three amendment's states were changed
- sometimes it updates if I reset all changes amendments to submitted
step to step reproduction:
- create motion with 200 words and 8 paragraphs (blindtextgenerator)
- Create 5 amendments (in line 1-2, 3-4, 5, 13-14 and 15), I deleted the first word for each amendment
- updating to the state to "accepted" or "not decided" (of any amendment) updates immediately, updating to "not decided" does not change the view automatically
(I called stop-dev and run-dev, the workflows are the default dev ones
|
@Elblinator showed me the problem on his computer. I build a small change, comparing the state_ids in the distinctUntil part. |
| for (const viewModel of updatedViewModels) { | ||
| viewModel.viewModelUpdateTimestamp = Date.now(); | ||
| } |
There was a problem hiding this comment.
This is done on model level (line 729) in base-repository.ts. Doing it here again is unnecessary. Remove it.
| @@ -47,7 +47,8 @@ export class AmendmentControllerService { | |||
| (prev, curr) => | |||
| prev?.length === curr?.length && | |||
| Math.max(...prev.map(e => e.viewModelUpdateTimestamp)) === | |||
| Math.max(...curr.map(e => e.viewModelUpdateTimestamp)) | |||
| Math.max(...curr.map(e => e.viewModelUpdateTimestamp)) && | |||
| prev?.map(e => e.state_id).equals(curr?.map(e => e.state_id)) | |||
There was a problem hiding this comment.
I guess the problem is, that entries get replaced by older entries while the array size stays the same. Probably it would be better to check for the ids of the entries.
prev?.map(e => e.id).equals(curr?.map(e => e.id))
There was a problem hiding this comment.
Also this should be done in motion-change-recommendation-controller.service.ts too.
Resolve #4303
Remove a strange distinct until changed timestamp logic which leads to failures in the issue.
I don't know, what the reason for this logic is, I think performance problems. So a whole test of the motion detail component is needed.