When re-saving a display of a view after setting the machine name (not edit, just open and click apply) the display will be deleted.

Steps to reproduce:
- Open an existing view that has an display mode.
- Click "Advanced" -> "Other" -> "Machine Name".
- Do NOT alter the machine name, but do click on Apply.
- Save the view.

Results in a “The requested page could not be found.” and a deleted view display.

CommentFileSizeAuthor
#5 2897576-5.patch2.1 KBLendude
#5 2897576-5-TEST_ONLY.patch1.03 KBLendude
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Ismaels created an issue. See original summary.

MustangGB’s picture

MustangGB CreditAttribution: MustangGB commented
Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.3.x-dev
Component: Miscellaneous » views.module
Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
Issue summary: View changes

I can reproduce this issue.

Don't know of any duplicates (there are other machine name issues, but not this one as far as I know or can find)

This might actually be Critical since it leads to data loss.

swentel’s picture

swentel CreditAttribution: swentel at eps & kaas for MuseScore commented

yeah, feels pretty critical to me too :)

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
Component: views.module » views_ui.module
Priority: Major » Critical
Status: Active » Needs review
FileSize
2897576-5-TEST_ONLY.patch1.03 KB
2897576-5.patch2.1 KB

This is where the logic goes wrong:

        $new_id = $display->display['new_id'];
        $display->display['id'] = $new_id;
        unset($display->display['new_id']);
        $executable->displayHandlers->set($new_id, $display);

        $displays[$new_id] = $displays[$id];
        unset($displays[$id]);

If new_id and id are the same unset($displays[$id]); will toss the display.

Test and fix. Bumping to critical for the data loss.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

I agree loosing that data is critical. Personally I was never a huge fan of renaming those display IDs, but people seem to really love that particular feature.

I'll wait to see the test only patch to get back red :) The fix looks like the right thing to do.

The last submitted patch, 5: 2897576-5-TEST_ONLY.patch, failed testing. View results

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Needs review » Reviewed & tested by the community

Nice!

  • catch committed 89bdb05 on 8.5.x 
    Issue #2897576 by Lendude, Ismaels: Resaving a view display results in...

  • catch committed 7ef9cbc on 8.4.x 
    Issue #2897576 by Lendude, Ismaels: Resaving a view display results in...

  • catch committed a09976e on 8.3.x 
    Issue #2897576 by Lendude, Ismaels: Resaving a view display results in...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Straightforward fix and good find. Committed/pushed/cherry-picked to all three 8.x branches.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Related issues: +#2915175: Duplicating a Page to Block after changing the display id and then saving the view destroys the Page

Continue here #2915175: Duplicating a Page to Block after changing the display id and then saving the view destroys the Page