spreadsheetimporter / ui5-cc-spreadsheetimporter

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

[Bug]: changeBeforeCreate Changed data not sent in payload #514

Closed ravish-garg closed 5 months ago

ravish-garg commented 6 months ago

OData Version

OData V2

Draft

not relevant

Scenario

Fiori Elements

Environment

VS Code

UI5 Spreadsheet Component

V29+

What happened?

Implemented handler for event changeBeforeCreate to manipulate the payload before sending to the backend, according to the sample implementation.

this.spreadsheetUpload.attachChangeBeforeCreate(function (oEvent) {
                    let payload = oEvent.getParameter("payload");
                    oEvent.getSource().setPayload(this._mapPropsToCopy(payload));
                }.bind(this), this);

The private method adds additional properties to the payload. However, in the actual POST request, none of the additional properties are sent.

This appears, due to the code not considering the correct variable for the updated payload. The setPayload() calls the method _setPayload, which updates a different variable.

      this.payloadArray = payload;
    },

Logic to trigger event and send Create:

            // Extension method to manipulate payload
            component.fireChangeBeforeCreate({
              payload: payload
            });
            this.createAsync(model, binding, payload);
          }

Relevant log output

NA

Spreadsheet Component Init

As per example repo

Manifest

No response

marianfoo commented 6 months ago

Hi @ravish-garg , thank you for opening the issue. Unfortunately i could not recreate your problem. I was able to manipulate and even add other properties. The code is correct and does not update a different variable. The variable payloadArray is used to loop over the data.

Can you please add more information, like data model and the excel file so i maybe can get the same error. Also, please use the newest version of the spreadsheet importer.

MARCxGAMBIT commented 6 months ago

Hey @marianfoo I think the documentation might be a bit misleading.

if (payload.price) {
        payload.price = Number(payload.price).toFixed(1);
    }
    oEvent.getSource().setPayload(payload);

the setPayload imo does nothing which is relevant for the upload. It only updates the payloadArray of the class SpreadsheetUpload (does that even make sense? payload is an object, payloadArray should be an array and is used to create the preview) (see here).

As @ravish-garg highlighted the following code is executed by the CC (see here):

try {
    await Util.fireEventAsync("changeBeforeCreate", { payload: payload }, component);
} catch (error) {
    // error handler
}
this.createAsync(model, binding, payload);

It appears that createAsync uses the exact same payload that was passed to changeBeforeCreate.

So @ravish-garg you have to modify the payload inplace in the event handler.

marianfoo commented 6 months ago

Hello to all, thanks for looking through my code. Yes it's true, the code is absolutely not clean and the naming is not ideal either.

The process worked here rather "accidentally". If the event parameter payload is manipulated, this is also used for the create as this is only a reference. I suspect that in @ravish-garg method _mapPropsToCopy the payload variable is not manipulated but a new object is created and passed to the setPayload method. The method setPayload does not work and my examples work because I always edit the reference directly. Therefore, in my examples, it would not be necessary to use the setPayload method at all. I am looking for a better solution.

A workaround is currently to only change the reference variable payload from the event parameters

marianfoo commented 5 months ago

fix was release with version 0.33.3, please check if that helped. here is the doc for it: https://docs.spreadsheet-importer.com/pages/Events/#example_1