Follow-up to #2204037: Views allows removal of required relationships and gives a fatal error on save

A site builder might have a view that is unfixable. The site builder won't be able to cancel the changes but would have to add the relationship back in order to fix the view.

Steps to reproduce if you apply the patch from #2204037: Views allows removal of required relationships and gives a fatal error on save:

  1. create a view to display a list of fields
  2. add a relationship
  3. add a field based on the relationship described above
  4. delete the relationship
  5. Cancel the changes

Otherwise, create an invalid View and try to cancel that, for example:

  1. create a View showing field
  2. remove all field from the View
  3. try to cancel

CommentFileSizeAuthor
#14 Screenshot_ofView_with_relatioship.png384.76 KBswetashahi
#10 2671182-10.patch1.76 KBlokapujya
#10 2671182-10-test-only.patch1.21 KBlokapujya
#8 2671182-8.patch567 byteslokapujya
#5 2671182-4.patch978 byteslokapujya
#3 dont_validate_on_cancel.patch877 byteslokapujya

Comments

lokapujya created an issue. See original summary.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Title: Views allows removal of required relationships and gives a fatal error on save » Views validates displays on Cancel
lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Status: Active » Needs review
StatusFileSize
new dont_validate_on_cancel.patch877 bytes

There is probably a much better way to do this...

Status: Needs review » Needs work

The last submitted patch, 3: dont_validate_on_cancel.patch, failed testing.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Status: Needs work » Needs review
StatusFileSize
new 2671182-4.patch978 bytes
lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Issue tags: +Needs tests

Wow, it passed. I don't know if it actually works though.

dawehner’s picture

dawehner
Primary language German
commented

Yeah I doubt that putting things into the FormBuilder is the right solution. Is there no way you could use #skip_validation_errors?

lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
StatusFileSize
new 2671182-8.patch567 bytes

I think it's actually as easy as this.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work

We use that in seven other places in ViewEditForm already for that exact reason (Duplicate, delete display, disable, etc)
Can we get some tests?

lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 2671182-10-test-only.patch1.21 KB
new 2671182-10.patch1.76 KB

Added basic tests.

The last submitted patch, 10: 2671182-10-test-only.patch, failed testing.

dawehner’s picture

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

Looks great! Thank you.

swetashahi’s picture

swetashahi commented
Issue tags: +#drupalconasia2016
swetashahi’s picture

swetashahi commented
Issue summary: View changes
StatusFileSize
new Screenshot_ofView_with_relatioship.png384.76 KB

I tested the issue using SimplyTest.me with latest patch on chrome browser.

Created two article content and added another content type called Issue type.
In the view added the fields and relationship as per the screenshot
image

Then after deleting the relationship and saving the view, got an error page saying "The website encountered an issue."

Marking needs work

swetashahi’s picture

swetashahi commented
Status: Reviewed & tested by the community » Needs work
lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community

The steps to reproduce assumed #2204037: Views allows removal of required relationships and gives a fatal error on save was in or that you had applied the patch there. Otherwise you just run into the issue described there.

If you want to reproduce this without that patch, you need to create an invalid display some other way. Added steps to the issue summary to do so.

This can go back to RTBC.

alexpott’s picture

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

Nice fix and nice test. Committed 886a81e and pushed to 8.0.x and 8.1.x. Thanks!

@swetashahi looks like you're in a good position to review #2204037: Views allows removal of required relationships and gives a fatal error on save

  • alexpott committed 0edf815 on 8.1.x 
    Issue #2671182 by lokapujya: Views validates displays on Cancel

  • alexpott committed 886a81e on 8.0.x 
    Issue #2671182 by lokapujya: Views validates displays on Cancel

(cherry...

Status: Fixed » Closed (fixed)

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