spreadsheetimporter / ui5-cc-spreadsheetimporter

A UI5 Component to integrate a Spreadsheet Upload for UI5 Apps.
https://spreadsheet-importer.com/
Apache License 2.0
84 stars 16 forks source link

Analyze popup when uploading data about loosing data #231

Open marianfoo opened 1 year ago

marianfoo commented 1 year ago

With #223 it was #214 fixed Need to analyze why this popup for loosing data activated. On V2 FE Non Draft 108: In Object Page, go to edit mode and change a line. Then upload a Excel file. A popup appears about loosing data. Apparently also happens on list report when uploading Excel files twice. Currently the popup is just turned off, but there is maybe and underlying issue or better solution for this. Maybe this is just the best solution.

wridgeu commented 1 week ago

Heyo, not a solution but maybe the input can help? Maybe not and I'm completely off base here. ^^

I had a look at 1.108 vs 1.96. In 1.96 (all V2 FE non Draft, like in the original issue) when you're doing the following sequence:

It leaves the edit mode but the table isn't refreshed with the newly added entry, oddly enough, I also couldn't see something in the getPendingChanges method of the default (unnamed) model. If you enter edit mode again, randomly press save (though you haven't, technically at this point, done any more change, the model updates!) the table displays the uploaded entry.

In 1.108 or the latest even, doing the same looked alright? Meaning, if I had one demo entry and added 4 more entries using the spreadsheet uploader, there were now 5 entries in total, as expected.

In Object Page, go to edit mode and change a line. Then upload a Excel file. A popup appears about loosing data.

Not sure if this is just to reproduce the popup but in general this action/behavior in itself makes total sense to me and doesn't classify as a bug. By changing anything within the table before uploading, you're creating a pending change in the model and thus triggering the data-loss popup. Seems about right, doesn't it? You have a change, adding new entries may remove previous changes, thus, prompting the abort of the action itself?

Though you may argue that adding new entries via the spreadsheet uploader doesn't adjust/change the previous changes or entries in total and they will therefore remain in tact making the popup a "bug" in that sense. But is that always the case then? Err'ing on the safe side seems reasonable.

You have probably debugged here already multiple times, still: bHasChanges + bNeedsPopup would be great places for a breakpoint here in the DataLossHandler (coming from CommonUtils): Image

Apparently also happens on list report when uploading Excel files twice.

This I couldn't reproduce at all. Uploaded a few times the same things like a maniac in 1.120, 1.108 and 1.96 but aside from not properly updating the tables in some version(s) it seemed fine.

The way wouter described it in his initial issue definitely seems like a bug but I couldn't reproduce that. Except: Could that be the "not displayed but yet still created" row in my observation above? Then maybe I looked at the wrong model or pending changes? If so ... I guess some more playing around with the upload and observing application state and trying to actually trigger the popup would be needed.

Disclaimer: me commenting is no sign of urgency on this bug nor do I face the issue myself. I simply had some time on my hands, needed a distraction, and thought maybe I can help, so I had a look.