I needed my users to be able to delete authmap entries, so I created a view.

This needs some extra views integration. Whereas #2864647: Provide Views integration provides a 'join' from the users table, there was no uid field or 'relationship' to the users table. This means you can currently list the authnames along with a certain user. But you can't make a list of authmap entries that does anything useful. (Like, also display the user ID.)

This MR provides

  • the above addition to views integration so we can make useful views based on authmap entries
  • a delete link + delete confirm form
  • a default view at admin/users/authmap, tab named "External". If you don't have views installed, you don't get it. But it's not needed for other module functionality, so I guess that's OK.
  • Two permissions for 'view' and 'delete' authmap entries. used by the view and form. (It's a bit much maybe, but I guess it's still the best to give these two things their separate permissions.)

Comments / current issues:

  • Re. naming: note we effectively also own the authmap 'namespace'. I made use of that.
  • The view takes the provider as a contextual filter (argument) and doesn't display the provider name. I figure that's the way most people want to use it: display just entries for one provider.
  • I wanted to set the default view (when it gets no contextual filter) to display a summary of all existing possibilities (provider names) to choose from - which is standard Views functionality. However that presents an SQL error because it tries to include COUNT(authmap.) AS "num_records". I bet that's a matter of Core Views not being able to work with tables that have no single primary key field.
  • ...so I set "display all records". This means if you have multiple providers, you do not see which provider these records belong to (because the view is not displaying the provider)!
  • The breadcrumb in the delete confirm form is incorrect: "Home > Administration > People > External authentication links > External authentication links". The first points to admin/people/authmap, the second to admin/people/authmap/PROVIDER. I'm not fixing that until we agree the above is OK / how it should be fixed.
  • I think a post-update hook is the way to install this new view; I don't know anything else. Please set me straight (or fix) if not.

Issue fork externalauth-3211814

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:

About issue forks

Comments

roderik created an issue. See original summary.

roderik opened merge request !2

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Status: Active » Needs review
Related issues: +#2864647: Provide Views integration
svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented

Thank you for the MR!

When I try to review this change, I'm getting a fatal error when trying to render the authmap view though:

Exception: No entity type for field delete on view authmap in Drupal\views\Plugin\views\HandlerBase->getEntityType() (regel 718 van /app/web/core/modules/views/src/Plugin/views/HandlerBase.php).

Not sure if this is related to something in my test install or not. Will try to debug further.

If this feature gets merged, will this conflict with the View in Samlauth module? I guess not?

svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented
Status: Needs review » Needs work
svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented
Version: 8.x-1.x-dev » 2.0.x-dev
roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented

Thank you.

This shouldn't conflict with the View in the Samlauth module. (At least not as far as I know. I made copies of all files so they are independent, and I plan to remove the Samlauth view when starting to depend on a newer Externalauth module.

Re. the No entity type for field delete error:

  • After checking where the error occurs and its likely callers, I think I see where the error comes from. I didn't see it because I didn't test it on a multilingual site.
  • While writing this, I was confused / not thrilled by the code separation in EntityLink vs. its parent LinkBase: EntityLink contains code that I do want, and LinkBase contains entity related code that I do not want / cannot use / is causing the error that you are seeing because I am not explicitly working around it.

By now I think I have a good enough overview to make a D10 Core issue / merge request that will simplify this and create a 'link base class for simple entity-less tables'. Which will (in D10) enable removing the majority of code from AuthMapDeleteLink. But I'll only create that if someone promises to review it :-) and also, not immediately.

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Status: Needs work » Needs review

I didn't test this on a multilanguage site, but feel confident enough that the last commit at least improves things, so I'll dare to set Needs Review again.

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented

So actually...

  • I forgot a change: there was more 'parent entity code' that shouldn't be called, in renderLink(). Credit to @lorisbel for pointing that out.
  • This did conflict with the view in the samlauth module: the new 'field' (for the delete link) in this patch called authmap.delete would come in the place of samlauth's field. That breaks things somehow, because of the new link path (/admin/people/authmap/{provider}/{uid}/delete): samlauth's view freaks out because it cannot provide/see any value for {provider}. In conclusion: while this is not your problem, I hope we can rename authmap.delete to authmap.delete_link, which will make the samlauth view not break upon upgrading externalauth.

(Also I see that that field's plugin_id was wrong in the exported view config: I have no idea why the view's export made it entity_link_delete before. I'm chalking that up to some internal but harmless bug in Views code.)

svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented
Status: Needs review » Fixed

Thanks, this seems to work perfectly now.
I tested this in combination with the latest dev branch 8.x-3.x of samlauth, and there seems to be no conflict.

Status: Fixed » Closed (fixed)

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