Problem/Motivation

The CckFieldValues source plugin does not return all source properties. I found a bug in CckFieldValues::prepareRow, we probably want to compare the existing array source object by key and not by value.

Proposed resolution

Replace array_diff with array_diff_key.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#8 cckfield-values-array-diff-2272853-7.patch7.76 KBwebflo
#7 2336743-languages-profiles-46.patch17.66 KBwebflo
#4 cckfield-values-array-diff-2272853-4.patch7.74 KBwebflo
#4 cckfield-values-array-diff-2272853-test-only.patch6.94 KBwebflo
#1 bugfix-cckFieldValues-2272853.patch908 byteswebflo

Comments

webflo’s picture

webflo commented
StatusFileSize
new bugfix-cckFieldValues-2272853.patch908 bytes
webflo’s picture

webflo commented
Title: CckFieldValues loses duplicate values » CckFieldValues source plugin loses similar field values
benjy’s picture

benjy commented
Status: Needs review » Needs work

Can we have a test to prove the issue exists?

webflo’s picture

webflo commented
Status: Needs work » Needs review
StatusFileSize
new cckfield-values-array-diff-2272853-test-only.patch6.94 KB
new cckfield-values-array-diff-2272853-4.patch7.74 KB

Yes, here is a failling test and a fix.

webflo’s picture

webflo commented
Issue tags: +Amsterdam2014

Status: Needs review » Needs work

The last submitted patch, 4: cckfield-values-array-diff-2272853-4.patch, failed testing.

webflo’s picture

webflo commented
Status: Needs work » Needs review
StatusFileSize
new 2336743-languages-profiles-46.patch17.66 KB

Sorry wrong patch.

webflo’s picture

webflo commented
StatusFileSize
new cckfield-values-array-diff-2272853-7.patch7.76 KB
webflo’s picture

webflo commented

The last submitted patch, 7: 2336743-languages-profiles-46.patch, failed testing.

The last submitted patch, 4: cckfield-values-array-diff-2272853-test-only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 8: cckfield-values-array-diff-2272853-7.patch, failed testing.

Status: Needs work » Needs review

webflo queued 8: cckfield-values-array-diff-2272853-7.patch for re-testing.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

I walked through this test with @webflo at Amsterdam sprints and it works as intended. Tests check the right behavior and test-only patch fails at #4 as expected.

benjy’s picture

benjy commented

+1 this looks good to me.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed d111ef3 and pushed to 8.0.x. Thanks!

  • alexpott committed d111ef3 on 8.0.x 
    Issue #2272853 by webflo: Fixed CckFieldValues source plugin loses...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.