pimcore / data-importer

This extension adds a comprehensive import functionality to Pimcore Datahub.
Other
39 stars 59 forks source link

Resolver element loading strategy (attribute and path) not working #68

Closed bitfactory-justin-kouwenhoven closed 3 years ago

bitfactory-justin-kouwenhoven commented 3 years ago

Attribute

When use the resolver element loading strategy attribute option, any existing data objects won't get updates. Instead a new data object will be created.

Schermafbeelding 2021-07-22 om 15 39 51

Path

When use the resolver element loading strategy path option and using the mapping to use a column value as the system key, existing objects won't be updated. Instead it's trying to create a new object with the same path (with column value as key). The import creates an error log for each entry.

Schermafbeelding 2021-07-22 om 15 45 14

Also trying to open the log file object results in a error page:

Schermafbeelding 2021-07-22 om 15 46 12
fashxp commented 3 years ago

cannot reproduce. are your elements published? please also see https://github.com/pimcore/data-importer/issues/24

bitfactory-justin-kouwenhoven commented 3 years ago

@fashxp I removed all my data objects, then i changed the element publish option to always publish and started the import several times. This worked in combination with element loading attribute, the data objects are updated.

It did not work if i changed the element loading to path. The same error from above is still triggered.

fashxp commented 3 years ago

can you please further debug. all loading should happen here: https://github.com/pimcore/data-importer/blob/1.1/src/Resolver/Load/PathStrategy.php#L34

fashxp commented 3 years ago

@bitfactory-justin-kouwenhoven any updates?

fashxp commented 3 years ago

@bitfactory-justin-kouwenhoven ping

skoch-twocream commented 3 years ago

@fashxp via Mail [...] Ich würde es aber wie im Issue geschrieben nicht mit einer Checkbox lösen, sondern eine eigene Location Strategy erstellen – sollte eigentlich sehr einfach möglich sein. [...]

Please assigne me to this issue ;-)

fashxp commented 3 years ago

@skochTC didn't you mean this one: https://github.com/pimcore/data-importer/issues/73?

skoch-twocream commented 3 years ago

@skochTC didn't you mean this one: #73?

Oh... okay there are two todos for me :-D For this issue, I can reproduce it when the attribute of the resolver is the first column (index 0) in the XLS.

Is there a deadline for the next bundle release?

fashxp commented 3 years ago

we established a bi-weekly release cycle for bug fix releases ... next release would be on September 15th, if any bugfixes are available.

skoch-twocream commented 3 years ago

Sorry, but I don't think I'll be able to do that at work until the 15th. Privately I have this week also no time.

I scheduled this for me for the next week.

fashxp commented 3 years ago

@skochTC fyi see #90 .... a different topic, sorry..

skoch-twocream commented 3 years ago

After testing for a while with the new version, I can no longer reproduce this.

@bitfactory-justin-kouwenhoven - Can you try this again please?

skoch-twocream commented 3 years ago

@fashxp - Okay! Sorry I actually can reproduce it...

This is my selection of valid XLS header columns: image

The ExtJs-Store looks like this: image

I added a new listener to the select field to dump the current value:

https://github.com/pimcore/data-importer/blob/1.x/src/Resources/public/js/pimcore/configuration/components/resolver/load/attribute.js#L100

                        listeners: {
                            select: function(context) {
                                console.log(context.value);
                                console.log(context);
                                console.log(this.configItemRootContainer.columnHeaderStore);
                            }.bind(this)
                        }

This allowed me to find this out: image

The "valueField" defined in the Ext component does not work with the DataIndex "0":

                {
                        xtype: 'combo',
                        fieldLabel: t('plugin_pimcore_datahub_data_importer_configpanel_data_source_index'),
                        name: this.dataNamePrefix + 'dataSourceIndex',
                        value: this.data.dataSourceIndex,
                        store: this.configItemRootContainer.columnHeaderStore,
                        displayField: 'label',
                        valueField: 'dataIndex', // This field...
                        forceSelection: false,
                        queryMode: 'local',
                        triggerOnClick: false
                },

You can also see this in the save request: image

I am not that familiar with Ext.JS. Do you have any idea why this is?

skoch-twocream commented 3 years ago

After a few more test runs I could now find out that the problem is not with the index "0" but on the label. The question still remains, why the Ext.JS-Store stores the label as value and not the "dataIndex"?

In the XLS, the attribute looks like this: image

As you can see, there is a line break in the column.

fashxp commented 3 years ago

hmm, tried to reproduce this on my end ... also added the console logs ...

image

for me it works just fine.

skoch-twocream commented 3 years ago

Hmm I really have no idea... Attached is the XLS I used.

Preview_E-Com_MT_20210713_mit_Identifier(1).xlsx

Can you try that again with this? Otherwise it may be browser dependent?

I use Firefox as my default browser (93.0b9 (64-bit))

mcop1 commented 3 years ago

Hello,

I am sorry, but I can´t reproduce this issue on my end of the side, even if I use the provided XLSX file. Neither with Chrome nor with Firefox.

stg231178 commented 3 years ago

After a few more test runs I could now find out that the problem is not with the index "0" but on the label. The question still remains, why the Ext.JS-Store stores the label as value and not the "dataIndex"?

In the XLS, the attribute looks like this: image

As you can see, there is a line break in the column.

I tested this with an own excel file and i think its really a problem with skipping the first row. For every excel sheet i tried, the first row (labels) made it impossible to import the data below. Maybe someone should have a look at the skipping logic

fashxp commented 3 years ago

as already noted ... we cannot reproduce this on our end. we highly appreciate any PR to fix that.

skoch-twocream commented 3 years ago

@stg231178 - Which Pimcore version are you using?

I expect that this is an ExtJS issue. Which is possibly fixed in Pimcore 10?

https://github.com/pimcore/pimcore/pull/7824

Anyway, we solved the problem under Pimcore version 6 by removing the line break. This topic is hard to reproduce and debug.

fashxp commented 3 years ago

ahh of course ... that would make sense, since Pimcore 10 switched to ExtJs 7. We just tried Pimcore 10.

stg231178 commented 3 years ago

@stg231178 - Which Pimcore version are you using?

I expect that this is an ExtJS issue. Which is possibly fixed in Pimcore 10?

pimcore/pimcore#7824

Anyway, we solved the problem under Pimcore version 6 by removing the line break. This topic is hard to reproduce and debug.

Pimcore 10.0.8 Data-Importer 1.2.1

fashxp commented 3 years ago

hmm ... ok, could you then please debug a bit further? or is it reproducible on our public demo too?

stg231178 commented 3 years ago

hmm ... ok, could you then please debug a bit further? or is it reproducible on our public demo too?

at the moment i have the effect that it work with "skip first row" but i cant reproduce the failure anymore. But i will test it with new import configurations

fashxp commented 3 years ago

I'll close that for now. Feel free to create folluwup tickets when you have more insights. thank you very much!