Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Add the missing schema for views.relationship.flag_relationship.
Comment | File | Size | Author |
---|---|---|---|
#17 | splitOffSchemaFixes_0.patch | 7.12 KB | Berdir |
| |||
#6 | interdiff-add_views_schema_for-2555365-2-6.txt | 3.37 KB | edurenye |
#6 | add_views_schema_for-2555365-6.patch | 3.88 KB | edurenye |
#6 | add_views_schema_for-2555365-6-test_only.patch | 2.13 KB | edurenye |
#2 | add_views_schema_for-2555365-2.patch | 522 bytes | edurenye |
Comments
Comment #2
edurenye CreditAttribution: edurenye at MD Systems GmbH commentedAdded the views schema.
Comment #4
BerdirProbably needs tests, we need a test view that adds a relationship so we can test the schema.
Comment #5
socketwench CreditAttribution: socketwench as a volunteer commentedCould we leverage Flag Bookmark for the test view? It seems like a natural place.
Comment #6
edurenye CreditAttribution: edurenye at MD Systems GmbH commentedAdded the test and solved some errors that I found doing the test.
Comment #8
joachim CreditAttribution: joachim commented> Could we leverage Flag Bookmark for the test view? It seems like a natural place.
I'm not sure that's really giving us anything -- the test has to depend on the submodule. Do we even need a predefined view for this anyway?
Can we make this node programatically instead? We're not testing node module's UI.
I'm confused about this bit!
Comment #9
socketwench CreditAttribution: socketwench as a volunteer commented@joachim I think you meant to mark this as needs work?
Comment #10
Berdir3. you're confused because the test has nothing to do with bookmark the way it is. It alters the frontpage view. I just showed that to @edureyne as an example for manual testing, *not* to do that in an automated test :) That doesn't make sense.
I think that writing a test for flag_bookmark *does* make sense. Because it already has a view with exactly such a relationship, so just having a test that enabled flag_bookmark should already be enough to trigger the schema error? And it does, you can see that nicely in the failing test, it doesn't fail on those frontpage view changes, it already fails on importing the bookmark view.
Comment #11
BerdirCrosspost.
Comment #12
joachim CreditAttribution: joachim commented> I think that writing a test for flag_bookmark *does* make sense. Because it already has a view with exactly such a relationship,
True.
But it means that if we ever change the bookmark view for some reason, it affects our test.
Also, if a site builder alters that view, and uses core and contrib tests as part of their CI, then that could break their tests too.
I'm maybe worrying too much ;)
Comment #13
BerdirYes. But you can also add a safe guard by checking if the view has a relationship. But that *is* all the bookmark view is about. that relationship :) And we should have such a test anyway.
No. Tests always use default config.
Comment #14
edurenye CreditAttribution: edurenye at MD Systems GmbH commentedI will continue with this when this issue: https://www.drupal.org/node/2562971 is fixed. As it's not possible to activate this view now.
Comment #15
edurenye CreditAttribution: edurenye at MD Systems GmbH commentedComment #16
socketwench CreditAttribution: socketwench as a volunteer commented#2562971: Flag bookmark view is broken Is fixed, setting to active.
Comment #17
BerdirReuploading the patch from the referenced issue.
not sure if this is correct.
Comment #18
socketwench CreditAttribution: socketwench as a volunteer commentedIf it isn't, I don't immediately see a problem.
Comment #20
socketwench CreditAttribution: socketwench as a volunteer commentedSince this was previously reviewed in another issue, and the tests apparently pass, let's commit it.
Thanks!