- ๐ฌ๐ทGreece Pavel Ruban
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 three 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.
It's a pretty generic module and my guess that it's quite an often need to do Audi 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
- ๐ฌ๐ทGreece Pavel Ruban
> 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 โ
committed de32a81c on 3.0.x
Issue #3519161: Fix routes conflict with grequest, fix access checks...
-
pavel ruban โ
committed de32a81c on 3.0.x
- ๐ณ๐ฑNetherlands ekes
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.
-
ekes โ
committed b65ed86e on 3.0.x authored by
pavel ruban โ
Issue #3519161 by pavel ruban, ekes: Routes conflicts and access checks...
-
ekes โ
committed b65ed86e on 3.0.x authored by
pavel ruban โ
- ๐ณ๐ฑNetherlands ekes
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? - ๐ฌ๐ทGreece Pavel Ruban
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)
- ๐ฌ๐ทGreece Pavel Ruban
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
- ๐ฌ๐ทGreece Pavel Ruban
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.
- ๐ณ๐ฑNetherlands ekes
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?
- @pavel-ruban opened merge request.