Comments

Lund created an issue. See original summary.

drunken monkey’s picture

Component: General code » User interface
Status: Active » Needs review
Issue tags: +Release blocker
StatusFileSize
new1.62 KB
new3.45 KB

Thanks a lot for reporting this! You're completely right, we totally forgot to actually sort the processors when showing them in the form.
Please test/review the attached patch which should fix this!

I also included a test, but those don't seem to work anymore for me locally. Let's see what the d.o test bot has to say about it.

drunken monkey’s picture

OK, seems the error I get ("A non-numeric value encountered") was added in PHP 7.1, which I apparently now have. Could therefore easily be a "bug"/incompatibility in Core's testing framework.

borisson_’s picture

Testing your suspicion by adding a test w/ php 7.1 to #2

The last submitted patch, 2: 2846255-2--processor_form_order--tests_only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 2: 2846255-2--processor_form_order.patch, failed testing.

drunken monkey’s picture

Status: Needs work » Needs review
StatusFileSize
new831 bytes
new1.62 KB
new3.45 KB

Hm, suspicion not confirmed, damn. Then apparently some hiccup in my local test environment – but most likely still related to PHP 7.1, I'd say. Anyways, since tests have been less and less reliable for me locally since switching to BTB in any case, I guess I'll just have to rely on the d.o test bot more again.

This should fix the obvious errors in the test results above.

The last submitted patch, 7: 2846255-7--processor_form_order--tests_only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 7: 2846255-7--processor_form_order.patch, failed testing.

drunken monkey’s picture

Status: Needs work » Needs review
StatusFileSize
new2.18 KB
new2.55 KB
new4.38 KB

The last submitted patch, 10: 2846255-10--processor_form_order--tests_only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 10: 2846255-10--processor_form_order.patch, failed testing.

drunken monkey’s picture

Status: Needs work » Needs review
StatusFileSize
new3.57 KB
new2.55 KB
new4.41 KB

The last submitted patch, 13: 2846255-10--processor_form_order--tests_only.patch, failed testing.

drunken monkey’s picture

Attached the wrong tests-only patch version.

Status: Needs review » Needs work

The last submitted patch, 15: 2846255-13--processor_form_order--tests_only.patch, failed testing.

drunken monkey’s picture

Status: Needs work » Needs review

Looking good. Anyone want to test/review?

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, we use a similar method in the facet test suite, a similar implementation might make the test easier to read? http://cgit.drupalcode.org/facets/tree/tests/src/Functional/TestHelperTr...

Not blocking the commit on that though.

drunken monkey’s picture

Status: Reviewed & tested by the community » Fixed

Looks good, we use a similar method in the facet test suite, a similar implementation might make the test easier to read? http://cgit.drupalcode.org/facets/tree/tests/src/Functional/TestHelperTr...

I had something like that, too, but, as the comment explains, we can't be sure of the actual order since some processors have the same weight assigned and their order is therefore unpredictable. That's why I went with just assuring that the weight is never falling.
I guess the alternative would be to explicitly set a (different) weight for all the processors – but I wanted to avoid doing that, since it's just a giant $edit array. And, this way we would also spot any problems that only arise when two weights are the same.

Anyways, thanks a lot for reviewing!
Committed.

  • drunken monkey committed e4157a2 on 8.x-1.x
    Issue #2846255 by drunken monkey: Fixed displayed order of processors in...

Status: Fixed » Closed (fixed)

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