Problem/Motivation

Views which use the flag_relationship plugin need to depend on the flag that is used by the relationship.

Proposed resolution

Change relationship plugin to add the dependency.

Remaining tasks

Create patch.

User interface changes

N/A.

API changes

N/A.

Data model changes

Views which use the flag relationship plugin have a new dependency.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

socketwench created an issue. See original summary.

socketwench’s picture

Starting with a rollback of the last known good branch version.

Status: Needs review » Needs work

The last submitted patch, 2: rollbackTest_2757395.3.patch, failed testing.

The last submitted patch, 2: rollbackTest_2757395.3.patch, failed testing.

socketwench’s picture

Okay, that at least confirms that it wasn't Flag changes that broke the module. Something in core testing changed.

socketwench’s picture

Regenerating the bookmark flag to see what happens...

Status: Needs review » Needs work

The last submitted patch, 6: rollbackTest_2757395.6.patch, failed testing.

The last submitted patch, 6: rollbackTest_2757395.6.patch, failed testing.

Berdir’s picture

Are you using the latest 8.2.x version locally? I can reproduce them.

AdminUiTest Fails because of #2753055: Change "Save order" to "Save". You can just switch to Save but then it will fail in 8.1. You could detect which string is on the page and then use that dynamically to make it work with both.

Not sure about bookmark, maybe ask @dawehner about that one.

socketwench’s picture

Not sure about bookmark, maybe ask @dawehner about that one.

Me either. I regenerated the view yml on the most recent version of core, but it still fails with a similar error.

socketwench’s picture

Status: Needs review » Needs work

The last submitted patch, 11: brokenFlag82_2757395.11.patch, failed testing.

The last submitted patch, 11: brokenFlag82_2757395.11.patch, failed testing.

socketwench’s picture

This issue appears to have broken flag: #2754477: Unexpected config entity delete due to dependency calculations

Works before that commit, broken after.

jonathan1055’s picture

Hi,
In Scheduler module we had a similar thing where our previously fully-passing codebase on 24th June started failing in 8.2. #2757625: Previously passing tests now fail in Drupal 8.2 - Change assertRaw to assertText has the details.

In our case the cause was core issue #2513402: Any time a user creates a thing and saves, display a link to the thing created in the status. which was committed on 25th June.

Jonathan

alexpott’s picture

Title: Flag 8.x HEAD is broken » Flag 8.x HEAD is broken because views need to depend on the flag that provides the relationship
Status: Needs work » Needs review
FileSize
1.67 KB

The problem is that the order in which things are installed is not guaranteed when the config system can;t work out the dependencies. Views which use the flag_relationship plugin need to depend on the flag they are using.

alexpott’s picture

The patch in #16 also makes flag_bookmark re-installable because the config it provides now depends on it using the enforced dependency.

alexpott’s picture

Issue tags: +Needs tests

This patch needs to add tests to ensure the dependency is added. It also needs to re-export all the views it creates - and it should provide post update function that updates existing views that use the flag_relationship plugin.

Status: Needs review » Needs work

The last submitted patch, 16: 2757395-16.patch, failed testing.

The last submitted patch, 16: 2757395-16.patch, failed testing.

alexpott’s picture

The fails happening in #16 are happening because of #2758395: Flag 8.x HEAD is broken because 'save order' has changed to 'save'

Patch attached adds missing dependencies and tests the two feature modules provided by flag to ensure they can be reinstalled and what they create has the expected dependencies.

It would be nice to add tests for:

  • The update path
  • Use of the relationship plugin in the flag module - atm the tests depend on the feature modules

Status: Needs review » Needs work

The last submitted patch, 21: 2757395-21.patch, failed testing.

The last submitted patch, 21: 2757395-21.patch, failed testing.

socketwench’s picture

  • socketwench committed aeb1e58 on 8.x-4.x authored by alexpott
    Issue #2757395 by socketwench, alexpott: Flag 8.x HEAD is broken because...
socketwench’s picture

Status: Needs work » Fixed

Thank you Alex and everyone! HEAD is fixed again!

Status: Fixed » Closed (fixed)

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