Patch coming.

CommentFileSizeAuthor
#10 2629184-10.flag_.views-field-relationship.patch1.5 KBjoachim
#2 2629184.flag_.views-field-relationship.patch592 bytesjoachim
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

joachim’s picture

joachim CreditAttribution: joachim commented
Status: Active » Needs review
FileSize
2629184.flag_.views-field-relationship.patch592 bytes
zerolab’s picture

zerolab CreditAttribution: zerolab at Torchbox commented

Possibly unrelated, but had to apply this patch for #2629182: FlagViewsLinkField should output nothing when there is no flag, rather than 'N/A' to get it to respect the empty text.

  • joachim authored 5bbed2a on 8.x-4.x 
    Issue #2629184 by joachim: Fixed views flag link field won't work if the...
socketwench’s picture

socketwench CreditAttribution: socketwench commented
Status: Needs review » Fixed

Thanks!

joachim’s picture

joachim CreditAttribution: joachim commented
Priority: Normal » Major
Status: Fixed » Active

Reopening -- this caused #2655134: Flag link views field broken, tries to use flagging as the flaggable entity.

  • joachim committed 7e5eb7b on 
    Revert "Issue #2629184 by joachim: Fixed views flag link field won't...
joachim’s picture

joachim CreditAttribution: joachim commented

Reverted the commit of the patch above.

joachim’s picture

joachim CreditAttribution: joachim commented
Assigned: Unassigned » joachim

I think I've got this fixed locally; needs a bit of clean-up first.

joachim’s picture

joachim CreditAttribution: joachim commented
Status: Active » Needs review
FileSize
2629184-10.flag_.views-field-relationship.patch1.5 KB
socketwench’s picture

socketwench CreditAttribution: socketwench commented

The tests pass, but do we have a test that covers this specific case? What are the steps to reproduce the bug that caused the earlier commit needing to be reverted?

joachim’s picture

joachim CreditAttribution: joachim commented

Indeed, we have no tests covering this IIRC.

The things to check are:

- a view where the flag relationship is on the base. Eg a view of nodes with a flag view
- a view where the flag relationship is on another relationship. Eg a view of nodes with their authors and a flag on the authors. Or a view of users with their nodes and a flag on the nodes. (The first is a more realistic example; the second allows you to reuse the same flag ;)

socketwench’s picture

socketwench CreditAttribution: socketwench as a volunteer commented
Status: Needs review » Reviewed & tested by the community

...a view where the flag relationship is on another relationship. Eg a view of nodes with their authors and a flag on the authors.

That doesn't look possible at the moment? FlagViewsRelationship::buildForm() only pulls flags for the base's entity type.

The links on the base appear to work. It would be nice if we could augment FlagBookmarkUITest to actually use the remove bookmark link.

joachim’s picture

joachim CreditAttribution: joachim commented
Status: Reviewed & tested by the community » Needs work

> That doesn't look possible at the moment? FlagViewsRelationship::buildForm() only pulls flags for the base's entity type.

I don't think that's the case:

  public function buildOptionsForm(&$form, FormStateInterface $form_state) {
    parent::buildOptionsForm($form, $form_state);

    $entity_type = $this->definition['flaggable'];
joachim’s picture

joachim CreditAttribution: joachim commented
Status: Needs work » Reviewed & tested by the community

Whoops, didn't meant to change the status.

socketwench’s picture

socketwench CreditAttribution: socketwench as a volunteer commented

Maybe I'm not understanding how that use case yet. Here's what I was trying:

  1. Create a user flag "following".
  2. Build a view that selects content.
  3. Add a relationship that brings in the author's user data.
  4. Add the Flag relationship.

At which point there's no way to select the "following" flag, only flags on the content.

joachim’s picture

joachim CreditAttribution: joachim commented

> Add the Flag relationship.

Did you set the relationship for that relationship? And was is the 'User flag' relationship you picked in the 'Add relationship' dialog? (Note that that won't appear immediately due to #2629180: Add a test to ensure that views data cache is cleared when flag added/deleted.)

I have pretty much exactly that set-up and it works fine.

socketwench’s picture

socketwench CreditAttribution: socketwench as a volunteer commented

Ahhhh, you need to clear the views cache after saving the view, but before adding the flag relationship. Then the user flag relationship shows up.

This "followed authors" view would be a good example to add to Flag Bookmark. And, obviously, useful for adding UserFlagType test cases.

YoyoS’s picture

YoyoS CreditAttribution: YoyoS commented

I created a flag, used it and added a view field-> Flagging : Flag Link

This error shows when I want to display the page containing the view

( ! ) Fatal error: Call to a member function getLinkTypePlugin() on null in drupal8\modules\flag\src\Plugin\views\field\FlagViewsLinkField.php on line 100

Is it related to this question ?

joachim’s picture

joachim CreditAttribution: joachim commented

Not really -- could you file a new issue for that please?

YoyoS’s picture

YoyoS CreditAttribution: YoyoS commented

I filled a new one but I thought it was linked to this one because it's talking about Flag link and relationship

-> https://www.drupal.org/node/2661170#comment-10814200

joachim’s picture

joachim CreditAttribution: joachim commented

> augment FlagBookmarkUITest to actually use the remove bookmark link.

Not entirely sure what you mean, so could you take care of filing an issue for that please?

Filed #2661404: add a view to test flag link field on an entity on a relationship too.

  • joachim authored 00de0e0 on 8.x-4.x 
    Issue #2629184 by joachim: Fixed views flag link field won't work if the...
socketwench’s picture

socketwench CreditAttribution: socketwench at FFW commented
Status: Reviewed & tested by the community » Fixed

Okay, since a related issue was already posted to close the testing gap, let's get this fix in.

Thanks everyone!

Status: Fixed » Closed (fixed)

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