Display a loading bar for the merging process

[CarolineDenis]

No description provided.

[CarolineDenis]

@maxpatiiuk , why is loadingBar doesn't pick up the change of mergeId and therefore doesn't display the Status dialog?
It's like mergeId was never changing even if the setMergeId is triggered in the loading function...

[maxpatiiuk]

@maxpatiiuk , why is loadingBar doesn't pick up the change of mergeId and therefore doesn't display the Status dialog? It's like mergeId was never changing even if the setMergeId is triggered in the loading function...

in your useEffect, you set loadingBar to true on change to mergeId - you never set it to false (except if user clicks on the abort button), and so loading bar never disappears

[maxpatiiuk]

You don't need loadingBar state at all - it only adds complication

loadingBar should only be visible if mergeId is not undefined, right?

so just do this:

Solution

const [mergeId, setMergeId] = React.useState<string | undefined>(undefined);
// later:
mergeId === undefined
// and:
()=>setMergeId(undefined)

[CarolineDenis]

@maxpatiiuk, I am facing the same issue where mergeId state is not being updated

[CarolineDenis]

NOTES: the setError(undefined) on line 268 is not working neither.
Error is undefined in this case as we hope it to be but only because the original value is 'undefined'.
For the test we can do: setError('succeeded') and error will still be undefined.

[CarolineDenis]

@melton-jason @realVinayak if you have input on this..

[CarolineDenis]

NOTES:
Tried:
const wrapedSet = (value: string) => { setMergeId(value); };
added after const [aborted, setAborted] = ...
And call wrapedSet(response.data) in the .then after for (const clone of clones...)

==> mergeId is still undefined
==> break point on setMergeId(value) shows the value to be response.data like expected though...

[CarolineDenis]

NOTES:
mergeId is now being set to the proper data 🥳
I commented out
React.useEffect( () => (ids.length < 2 ? handleClose() : undefined), [ids, handleClose] );
to not have the component unmounted, the issue now is that we can open the merging with only 1 agent.
Could we display the MerginDialog component directly from RecordMergingLink with a conditional instead of displaying it from an overlay route?

[maxpatiiuk]

sorry, I just realized that I left comments but forgot to publish them 😭

[maxpatiiuk]

Could we display the MerginDialog component directly from RecordMergingLink with a conditional instead of displaying it from an overlay route?

No. Reasons:

• By being on a separate URL, record merging is not dependent on query builer - you can call is easily from the Form Meta menu or the express search
• It has it's own URL, so it's integrated with the browser history - users can use back/forward arrows to close/reopen the dialog, as well as look in their browser history to see which records they merged
• Third-party devs could create tools for finding duplicate records, and then just linking to the record merging tool's URL - we don't provide proper ways of finding duplicates at the moment, but members already created some tools for finding duplicates, and they would appreciate not having to re-create the merging UI itself but just be able to easily reuse our interface

[maxpatiiuk]

mergeId is now being set to the proper data 🥳
I commented out
React.useEffect( () => (ids.length < 2 ? handleClose() : undefined), [ids, handleClose] );

I don't like this solution at all. Your mergeId issue could have been caused by any of the other things I left a comment about
If you resolve comments, and it's still not working correctly, ping me and I will look into fixing this

[grantfitzsimmons]

Merge this as soon as we can!

[bronwyncombs]

The progress bar doesn't appear for quick merges

Screen.Recording.2023-08-12.at.10.44.24.AM.mov

---

Bigger merges do get the progress bar after a loading dialog.

However, it will tell you success but then send a fail notification

https://drive.google.com/file/d/1zehhB45CYmOHkdRnEZ_vK3JtxUz90NCz/view?usp=sharing
(drive for privacy)

---

Also, the progress bar time countdown will display with dashes when resized:

Maybe move it to the top or bottom instead of the side?

https://drive.google.com/file/d/1-BGoZFdP0lktlbZ1qybW10adoySrK9Q-/view?usp=sharing

[melton-jason]

The progress bar doesn't appear for quick merges

Screen.Recording.2023-08-12.at.10.44.24.AM.mov
Bigger merges do get the progress bar after a loading dialog.

This is not for quick merges, only when merging exactly two agents

However, it will tell you success but then send a fail notification

https://drive.google.com/file/d/1zehhB45CYmOHkdRnEZ_vK3JtxUz90NCz/view?usp=sharing (drive for privacy)

This is a backend-related problem. The backend endoint for getting the merge status always states the merge is a success. Should be fixed if we fix that endpoint

[melton-jason]

Can this be tested again?
The bug causing the dialog to close immediately after beginning the merge with exactly two agents should be fixed.

And the remaining time should now also be displayed below the loading bar

[bronwyncombs]

I get the correct dialog when merging two agents now! Remaining time doesn't show though

Screen.Recording.2023-08-14.at.11.51.48.AM.mov

[CarolineDenis]

I get the correct dialog when merging two agents now! Remaining time doesn't show though

Screen.Recording.2023-08-14.at.11.51.48.AM.mov

if you increase the height of the dialog can you see it? Or also sometime if the merge is too fast there is no remaining time showing up

[bronwyncombs]

@CarolineDenis, resizing the dialog has no change

Screen.Recording.2023-08-14.at.1.40.24.PM.mov

[CarolineDenis]

Thank you for checking,
then I would say it's because it was too fast

[melton-jason]

Can this be merged after bronwyn approves it?
As far as I know, the only remaining problem that needs to be addressed related to this is to fix the bug where the backend returns a SUCCESS when calling the merge status endpoint even when the merge fails.
However, this PR has gotten quite lengthy and the fix for that should probably be its own PR, in my opinion.

[bronwyncombs]

Still not seeing the time remaining

Screen.Recording.2023-08-15.at.11.05.03.AM.mov

[melton-jason]

That part was not addressed.

Here's the problem:

• When the Proceed/Merge button is clicked, the backend begins merging

• The frontend (due to a limitation of us being on HTTP 1.1) can only have 6 concurrent requests at a time (See Add HTTP2 (and HTTP3) support in NGINX containers #2608)

• Thus the merging can complete on the backend before the request to fetch the status (and thus the progress/time remaining) ever resolves

• On the frontend, it can not estimate the time remaining or progress without either making assumptions (which may be highly inaccurate) or actually receiving a response from the backend while the agents are in the process of merging

  • In other words, the frontend goes straight from starting the merge to fetching the status, which is now finished

Essentially, the frontend fetches the merging status from the backend in "chunks".
To solve this and have an accurate time of mergering (and other asynchronous operations occuring on the server) we would either need to implement websockets which allow a two-way continuous stream of data directly from the frontend and backend or upgrade to HTTP 2+ which would alleviate the problem by allowing us to send/receive more concurrent requests

[CarolineDenis]

@melton-jason @bronwyncombs
added a time out in the back end for testing and time remaining is displayed 👌