Problem/Motivation

I use drupal core 10.4.6, group 3.3.3, grequest 3.2.2

This module alpha 1

I guess the issues I experience partially may be due to group module logic update (plugin naming convention for node bundles) and partially due to usage of this module and grequest at once

My use case - we have a group which provides ability for users to join as a member, group itself is a building / site / area coworker space which people may book, booking should be moderated, so we use node booking bundle and gnode request module to achieve this. If user is a member the group updates, news, space availability will be sent by email.

When I installed the node request module the moderation view was missing even with all the roles and permissions properly set, views links to approve reject nodes didn’t work even when I tried to add those to a custom view

After debugging I found out that:
* menu local task alter has issue in logic and always remove moderation view
* approve / reject entity links routes are equal to grequest module so they are overridden and unavailable
* after fixing routes entity view links still were missing as inside of the access check logic there was a similar plugin access check as with menu local task

After fixing all the issues node moderation started to work as expected for me

CommentFileSizeAuthor
#9 3519161-9.patch6.42 KBpavel ruban
#3 3519161-3.patch2.98 KBpavel ruban
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:

Comments

pavel ruban created an issue. See original summary.

pavel ruban’s picture

StatusFileSize
new2.98 KB

for 3.0.0-alpha1

pavel ruban’s picture

Status: Active » Reviewed & tested by the community
ekes’s picture

Hi,

Thanks for this, it's quite likely there are issues with grequest (as lots of the code came from there). I'll try and get to test it soon. One thing I noticed immediately though. Could you not use {$plugin}->getBaseId() in place of getPluginId() and then looking for the str_starts_with?

  • ekes committed 6dd36a98 on 3.0.x
    Issue #3519161 by pavel ruban: Avoid route name clash with grequest
    
pavel ruban’s picture

Hi,
Grequest code is valid as it's bound to users and plugin id behaves differently in there. Also as grequest and gnoderequest previously defined the same machine name routes for entity links if you install both modules at once one or other module will override routes depending on modules weight and when access tried to check the route result actually others module controller was called and provided false check result when it was expecting the access object interface.

Regarding the starts with, can you elaborate what is the concern about it?

In grequest you have one id for the plugin in group nodes there are no such a plugin and it has pattern gnode_request:article, gnode_request:basic_page & so on, depending on bundled enabled in the group node settings, so previous code didn't pass any access checks at all due to this, so now I check and if there is at least one plugin which starts with node request prefix I pass the check, mb group nodes interface was changed a bit and before it was the other way, it's why I mentioned it in the description.

pavel ruban’s picture

Ah, the 'could you not use' confused me, I guess you are fine with the approach just asked to use base id callback instead of manual check, yes sure, makes sense

pavel ruban’s picture

StatusFileSize
new6.42 KB

Refactored, adjusted logic a bit to properly handle plugin ids group_node & group_node_request conditions,

Please note that I found another fatal & fixed it:
https://git.drupalcode.org/project/gnode_request/-/merge_requests/2/diff...
https://git.drupalcode.org/project/gnode_request/-/merge_requests/2/diff...

request form render caused before:

Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException: The "group_relationship" entity type did not specify a "group-request-node" form class. in Drupal\Core\Entity\EntityTypeManager->getFormObject() (line 212 of core/lib/Drupal/Core/Entity/EntityTypeManager.php).
Drupal\Core\Entity\EntityFormBuilder->getForm() (Line: 91)
pavel ruban’s picture

rebased & fixed conflicts (amended the existing commit) with 3.0.x due to #6

ekes’s picture

Issue tags: -groups, -grequest, -views, -access control, -RouteNotFoundException +ddd2025

Adding ddd2025 because that's where I'm reviewing.
Note about which tagging https://www.drupal.org/node/3156530#s-what-to-use-issue-tags-for

ekes’s picture

I'm wondering about the form group-request-node. I notice I'd made a comment in the test that the module should probably use the default group generated forms.

Did you just have the error when the module presently called this form.
Or is there a good use for using a different form?

  • ekes committed b65ed86e on 3.0.x authored by pavel ruban
    Issue #3519161 by pavel ruban, ekes: Routes conflicts and access checks...
ekes’s picture

Status: Reviewed & tested by the community » Fixed

I've removed that form. If it has value we should remake it so that it asks which group to add the entity to - I think?

I found more grequest overlaps so fixed them at the same time, and added a couple of tests.

I'm not 100% that they've all be cleaned up. So I'll roll out another alpha.

If you spot some more do open another issue, MR.

It helps, me at least, if you keep the MR with multiple individual commits, for each issue/problem/thing-being-fixed, rather than as one bigger commit.

  • pavel ruban committed de32a81c on 3.0.x
    Issue #3519161: Fix routes conflict with grequest, fix access checks...
pavel ruban’s picture

> I'm wondering about the form group-request-node.

In my case I use this form to render it on the group page itself, same way as you do in all other places in your module, get a node instance and create request form with the gnode relation entity programmatically. Why not to keep the form? If you try to render the form so user can request moderation of a new node membership via entity form before it threw the fatal error as the entity metadata had no such a form entry, so how it was possible to render a form at all? What was a way to actually create a moderated request?

With the form you deleted you could programmatically eg grab the node from a route context or create a new node on the fly (in my case I create booking node instance with all the fields you configure at the fieldable group content entity api, so I gather date the user wants to book a group entity for, in submit create a node, fill it's fields from the input and create moderation request).

Eg same way you could configure multiple fields on content entity field and form display settings page, and render it eg in a sidebar of a node full/teaser view modes with the possibility to request the page's node membership. Now if the form doesn't exist is it possible to use fieldable entity form to make a membership request form? What is the workflow to add nodes to the group with moderation? Why it's necessary to select a node from the nodes list on the membership request form? I guess it's more naturally to render form in context of some node. If you want to select node you can create multistep form with node selection and then reuse the same entity form with node context from the previous step.

pavel ruban’s picture

Just saw your commit comments.

The form as a view field to add a node from a nodes list with a group selection is a good and useful which of course can be implemented and good to have but that wasn't my objective

The other scenario that is useful and which I needed and did is the ability to request some service bundle nodes membership right on the group page or groups list page , such nodes are generated on the fly upon submit with the user input context like bookings with desired dates intervals / people quantity, then a booking node appears in moderation view, admin approves / rejects it and approved bookings can be seen in a content view. So we can render booking calendar, review available seats , time slots etc. And we can configure any needed context fields on content plugin gnode request relation entity as needed.

Other possible scenarios e.g.: buying tickets with approve, or groups can be jobs and people can make offer with some details like price and ETA they can accomplish such a job and a person who provided the job can review candidates and select the most relevant one. Still all such users can request user group membership on top of nodes membership to receive newsletter, ads about the explicit group etc.

Gnode request is a pretty generic module and my guess that it's quite an often need to do such a things like above and it's better to have this possibility rather than limit module usage only to explicit cases like adding nodes to groups from node full / list pages with a group selection

Hope this explanation makes it's more clear now

ekes’s picture

I still don't understand how this differs from the form that Group automatically makes like at for example `/group/1/content/create/group_node_request%3Aarticle` and `group/1/content/add/group_node_request%3Aarticle`. But if (re)using these forms doesn't do what you need. If you could open a new feature request issue, and describe what is needed that is different from those forms I think that would help.

Status: Fixed » Closed (fixed)

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