angus
November 8, 2021, 6:10am
22
Note that I’ve made an updated draft PR with a new approach (incorporating changes requested by @david on my last attempt). As mentioned in the comments to the PR, I’m looking to finish it off sometime this week.
discourse:main ← angusmcleod:group_membership_via_authentication
opened 06:06AM - 08 Nov 21 UTC
Update on #12446.
@davidtaylorhq A draft for an update on the associated grou… p membership work. It's been rebased, brought in line with the latest changes and updated in the ways explained below. Let me know if you're onboard with the approach so far. I'll finish it off next weekend.
### Authentication and group retrieval
I've removed the secondary auth stuff, added a google-hd-specific boolean setting, and moved the group retrieval into a new google oauth2 omniauth strategy. I've made a new folder and added a spec for the strategy (something new in the codebase) as I thought it deserved a spec (which is also included). If we add in secondary auth to this strategy that too will need a spec. Some of the existing discourse omniauth strategies, e.g. Discord, may benefit from being seperated out and tested too.
### Group Provision and UI
As suggested, I've added a ``provides_groups?`` to the ``Auth::Authenticator``. Using this approach led to a new groups guardian method to use at the various group CRUD points. Speaking of which, I've added in support for group_associated_group creation in group creation (i.e. ``/g/custom/new``). Supporting it in both ``new`` and ``manage`` (update) also led to adding a new site attribute ``can_associate_groups``. I attempted to make this more group-route specific, however this leads to various clumsy workarounds to determine whether an admin can associate groups in ``/g/custom/new``. The site attribute approach proved the simplest and most performant (i.e. fewer ajax calls).
### Data model
I've made one change to the data model so far, namely to use ``after_commit`` hooks rather than ``after_create``/``after_destroy`` as using those hooks leads to duplicate record creation in ``Admin::GroupsController`` ``create``. I've yet to deal with the edge case you pointed out, i.e. multiple UserAssociatedGroups which link to the same group.
3 Likes