I apologize in advance, because this is probably a dupe... when I searched, I did find #1785560: Remove the sorting of configuration keys which claims this is already fixed, but it is not in my experience... and #1608912: (Re-)adding keys to configuration breaks the intended goal of being able to diff files which I can't tell if that's the same issue or not.

Any-who. Steps to reproduce:

1. Copy your active directory contents to your staging directory: cp sites/default/files/config_XXX/active/* sites/default/files/config_XXX/staging/
2. Add a new views.view.fruit.yml file in staging, and paste in the contents of fruit.txt (which is just a regular, run-of-the-mill view w/ a page display created with the page wizard). vi sites/default/files/config_XXX/staging/views.view.fruit.yml
3. Go to admin/config/development/configuration and see that is says you have stuff to import. If you "View differences" you'll see it's a new file. Import it.
4. Upon refresh it says you still have things to import. If you "view differences," it is because the page display has changed order. Import it again.
5. Finally it thinks everything is imported. Hooray.

CommentFileSizeAuthor
#9 drupal-display_zero_test_and_2b-2153469-9.patch1.79 KBswentel
#6 Screen Shot 2014-01-22 at 4.35.06 PM.png39.82 KBGarrett Albright
#5 drupal-display_zero_test_only-2153469-5.patch1.15 KBGarrett Albright
#5 drupal-display_zero_test_and_2b-2153469-5.patch1.75 KBGarrett Albright
#2 2153469-2B.patch614 bytesolli
#2 2153469-2A.patch802 bytesolli
fruit.txt3.49 KBwebchick
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

olli’s picture

olli CreditAttribution: olli commented 
display:
  page_1:
...
    position: 1
...
  default:
...
    position: 1

I'm not sure if I should file another issue for this, but I think this happens due to a problem when creating the view: the master display gets position 1 instead of 0.

olli’s picture

olli CreditAttribution: olli commented
Status: Active » Needs review
FileSize
2153469-2A.patch802 bytes
2153469-2B.patch614 bytes

Looks like the master is added twice.

If you apply either of these, re-create and export your fruit view, and try to reproduce the steps, then the bug in step 4 does not occur. It kind of fixes (or hides) this issue, so I'm not sure. Should we instead try to keep the original order of displays?

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented

The 2B patch fixes the problem. Now the default display is created with position 0.

Regarding the 2A patch: This change would mean that newDisplay() isn't used anymore to create the Master Display. I am not sure yet if that matters.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Needs work
Issue tags: +Need tests

Nice catch @olli I think A is the correct approach but can we add test to prove the regression?

Garrett Albright’s picture

Garrett Albright CreditAttribution: Garrett Albright commented
Status: Needs work » Needs review
FileSize
drupal-display_zero_test_and_2b-2153469-5.patch1.75 KB
drupal-display_zero_test_only-2153469-5.patch1.15 KB

Here's a test, both with and without patch 2B.

I don't really get how 2A would "work" unless there were some magic that creates displays if they're requested but don't exist…

Garrett Albright’s picture

Garrett Albright CreditAttribution: Garrett Albright commented
FileSize
Screen Shot 2014-01-22 at 4.35.06 PM.png39.82 KB

Here's a test, both with and without patch 2B.

I don't really get how 2A would "work" unless there were some magic that creates displays if they're requested but don't exist…

EDIT: When testing with the fruit.txt file, you'll see that the site will still report differences after updating, but if you check out the diff, they seem to have to do with equal but not identical values; string 10 vs int 10, empty strings vs false, etc. I'm guessing it's due to changes in configuration exporting since this issue was created.

Status: Needs review » Needs work

The last submitted patch, 5: drupal-display_zero_test_only-2153469-5.patch, failed testing.

olli’s picture

olli CreditAttribution: olli commented
Status: Needs work » Needs review
swentel’s picture

swentel CreditAttribution: swentel commented
Issue tags: -Need tests
FileSize
drupal-display_zero_test_and_2b-2153469-9.patch1.79 KB

re-rolled

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

I wonder whether we can also check the actual config object and the order in there?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Seems legit

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented

9: drupal-display_zero_test_and_2b-2153469-9.patch queued for re-testing.

alexpott’s picture

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

Committed 2421c75 and pushed to 8.x. Thanks!

  • Commit 2421c75 on 8.x by alexpott: 
    Issue #2153469 by Garrett Albright, olli, swentel, webchick: View page...
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Awesome, thanks! This has come up on CMI demos for the past several months. Can't wait to do my next one with alpha11/beta1 ;)

Status: Fixed » Closed (fixed)

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