search

symphonists / uniondatasource

A union datasources allows you to combine multiple datasources to output a single datasource for the primary purpose of a unified pagination.
13 stars 11 forks source link

Duplicating of ds-params (still) #42

Closed moretaste closed 7 years ago

moretaste commented 9 years ago

I'm still not convinced that https://github.com/brendo/uniondatasource/issues/40 is solved.

I understand that parameters doesn't have to be unique. But that isn't the problem.

The problem is that the parameters are duplicates (maybe the word repeat wasn't the right one). The more entries I make the more duplicates I got.

bzerangue commented 9 years ago

+1

I am having the same issue with Union Datasource with Symphony 2.5.1

brendo commented 9 years ago

Is this causing any issues other than visual?

bzerangue commented 9 years ago

Is this causing any issues other than visual?

If you mean by visual, it outputting or duplicating the same parameters. I don't if it is doing anything else that I can see.

params

params xml

moretaste commented 9 years ago

As far as I can see it is only visual, maybe a little influence on performance.

This happens after this commit: https://github.com/brendo/uniondatasource/issues/35 (which was indeed neccesary to make union datasource working in 2.5)

nilshoerrmann commented 9 years ago

I just encountered this bug as well and it's caused by this line: https://github.com/brendo/uniondatasource/commit/f5c226d08762608959815b9d75487262e4561fd3#diff-1ba9d55430a6916bdaf92be9ae651b2bR1068

The extension duplicates all output parameters that have not been an array before. The bug is breaking is breaking sites that rely on comparing parameter values because strings become arrays and thus the XML structure changes.

What's that line doing?

nilshoerrmann commented 9 years ago

Uncommenting that line fixes the issue for me (Symphony 2.6.2).

moretaste commented 7 years ago

Changing line https://github.com/brendo/uniondatasource/commit/f5c226d08762608959815b9d75487262e4561fd3#diff-1ba9d55430a6916bdaf92be9ae651b2bR1068

array_merge_recursive to array_replace_recursive seems to solve it. (Not yet fully tested)

nitriques commented 7 years ago

@moretaste wanna send a PR ?

brendo commented 7 years ago

Closed by #47, released as 1.2