To reproduce:

- start with a site with no user flags (such as on initial install of Flag)
- add a view of users
- notice there is no 'user flag' relationship, because there are no user flags yet
- add a user flag
- go back to the view and add a relationship: there is still no 'user flag' relationship, even though there is a user flag now.
- clear caches
- user flag relationship is now visible

Adding or deleting a flag should therefore clear Views data caches. For bonus points, only clear them when we're adding a flag of a type that doesn't exist yet / deleting the last flag of its type.

CommentFileSizeAuthor
#12 2926180-12.patch1.93 KBarunkumark
#5 interdiff-2-5.txt509 byteszahord
#5 2926180-5.patch1.51 KBzahord
#2 2629180-2.patch1 KBzahord

Issue fork flag-2629180

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

joachim created an issue. See original summary.

zahord’s picture

zahord commented
StatusFileSize
new 2629180-2.patch1 KB
zahord’s picture

zahord commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 2629180-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

zahord’s picture

zahord commented
StatusFileSize
new 2926180-5.patch1.51 KB
new interdiff-2-5.txt509 bytes

updated

zahord’s picture

zahord commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: 2926180-5.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

socketwench’s picture

socketwench commented

There is an older patch for this which contains the fixes for the Flag entity: https://www.drupal.org/project/flag/issues/2851054#comment-12412982

+++ b/tests/src/FunctionalJavascript/FlagContextualLinksTest.php
@@ -157,6 +157,7 @@ public function setUp() {
+    $this->container->get('views.views_data')->clear();

I'm still concerned about this line, though. Why are we clearing the views cache in a test?

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +Needs tests, +Needs reroll

This issue should be focused on adding tests cos I failed to write ajax tests for views relation in #2851054-12: Cannot add Flags relationship to views. Getting AJAX error

+++ b/tests/src/FunctionalJavascript/FlagContextualLinksTest.php
@@ -157,6 +157,7 @@ public function setUp() {
+    $this->container->get('views.views_data')->clear();

this no longer needed cos flags do clear cache on save/delete now

andypost’s picture

andypost
he/him
Primary language Russian
commented
Title: adding a flag should clear the Views data cache » adding a test that makes sure cache cleared when flag added deleted

case for test still valid in IS

socketwench’s picture

socketwench commented
Priority: Normal » Critical

Bumping status since this is holding up other views improvements.

arunkumark’s picture

arunkumark
Location Coimbatore
commented
Status: Needs work » Needs review
StatusFileSize
new 2926180-12.patch1.93 KB

I have re-rolled the patch to resolve the issue with the suggestion from the comment #10

Status: Needs review » Needs work

The last submitted patch, 12: 2926180-12.patch, failed testing. View results

socketwench’s picture

socketwench commented

Thanks for the patch @arunkumark, but the postSave() and postDelete() are already part of the Flag entity.

@andypost Do we still need the views clears in the tests?

andypost’s picture

andypost
he/him
Primary language Russian
commented

If was commited as I remember, here we should add a test and remove manual cleaning from otger tests

andypost’s picture

andypost
he/him
Primary language Russian
commented

Yes, cache clear in https://cgit.drupalcode.org/flag/tree/src/Entity/Flag.php?h=8.x-4.x#n503

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Title: adding a test that makes sure cache cleared when flag added deleted » Add a test to ensure that views data cache is cleared when flag added/deleted
Category: Bug report » Task
Priority: Critical » Normal
Issue tags: -Needs tests

That sounds like a normal task and not a critical bug then now? Also can't find any manual views data cache clears in test, being able to remove those would have been easy test coverage.

ivnish’s picture

ivnish
he/him
commented

Up

ivnish’s picture

ivnish
he/him
commented
Version: 8.x-4.x-dev » 5.x-dev

deaom made their first commit to this issue’s fork.

deaom opened merge request !163

deaom’s picture

deaom commented
Status: Needs work » Needs review

Updated the UserFlagTest to check that the relationship is not present on initial install and is present without additional cache clear after the user flag is set.

  • ivnish committed 0d8e59e6 on 5.x authored by deaom 
    test: #2629180 Add a test to ensure that views data cache is cleared...
ivnish’s picture

ivnish
he/him
commented
Status: Needs review » Fixed
Issue tags: -Needs reroll

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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