-
maxwellkeeble →
committed 7b86f273 on 1.0.x
Issue #3307887 by Nigel Cunningham, maxwellkeeble:...
-
maxwellkeeble →
committed 7b86f273 on 1.0.x
(1) Fully agree with needing this ability, was not envisioned as part of writing this. See below proposed approach.
Re: (2) - access checks are not just routing. They also need to be done on the element level, and the transition option level, for webforms generally and for this module to provide a good UI for selecting available transitions. I know in the other issue you raised (now resolved) it could be done in routing, but here the user can have full access to the route, but not necessarily access to every workflow element, and for each element they do have access to, not necessarily access to every transition. It needs to be (able to be) checked at some point separately from routing.
Currently this check in runTransitionOnElementValue prevents someone programmatically running a transition via this helper function that the user does not have access to, as you say. We can easily make that an optional access check, to provide an easy way to safely run the transition, or 'unsafely' run it programmatically if needed.
So I think the simplest compromise right now is to add an optional parameter to the function, $access_check, which is by analogy to how entity queries can disable the access check. Here, it will default to checking access, to avoid unintended consequences, but allow overriding.
If at some point we identify every possible case where runTransitionOnElementValue is used and ensure there is an access check of some sort in front of it, I think it's still good DX to have an optional access check in this function for doing things programatically, so we could just change the default for $access_check to false.
Anyway. I've added $access_check in the latest dev for now. Happy to discuss as I'm sure I've missed something, development of this module is in fits and starts due to my workload and my planned approaches may have drifted off a bit.
- Status changed to Needs review
over 1 year ago 10:29pm 15 February 2023 - Status changed to Fixed
about 2 months ago 7:10pm 24 September 2024 $access_check provided as optional parameter. Did not apply merge request for reasons stated, and this approach covers off the first use case.
2) Access checks should be done as part of route access checking anyway. This allows menu items that shouldn't be accessible to a user to be hidden and access prevented, rather than requiring additional code to say "Sorry. We showed you this menu item but you can't actually use it." It also makes the code better structured and more maintainable.
As mentioned we need to be able to access check at different points, not just at the routing level. Without this approach, it'll end up less maintainable as we'd need to repeat the access check logic across functions I think.
Automatically closed - issue fixed for 2 weeks with no activity.