🇬🇧United Kingdom @jonathanshaw

Note to self: I'm having doubt about the name "administer host registration". Need to consider naming carefully.

By the time this is released we could have close to 1,000 installs. Even if only 5% use an own permission varying by type, that is 50 sites that we have broken by removing functionality without a replacement. I don't want to do that. I don't have an alternative yet. Let's mull this over further.

Spitballing:
1. Add a hook_requirements warning now, wait until a major version release to remove. We can see how many people show up in the issue queue.
2. Keep them, mark them deprecated, add a hook_requirements warning, don't plan to do anything about it for years.
3. Create a legacy module that provides the permissions and makes them work as before. Make it a seperate project, not a child module, so you get stats on its usage and can eventually stop maintaining it. Or even mark it as not maintained, give them the responsibility.
4. Decide that we are not breaking anyone's site, they don't have to upgrade. Maybe make it a major version and keep backporting bugs to the previous version.
5. Keep the permissions around, but rename them "administer $type host registration settings" and mark them deprecated.
6. Create a "legacy" submodule that the sitebuilder enabled in an update hook if the site needs it.

I think it would be best to have a way of thinking that enabled breaking changes to be made to the module.

But what about this: the $context (and things derived from it) CAN be used in hook_entity_create_access_alter, with no way to broadcast that fact back to the AccessControlHandler. (And it may add the need for additional caching contexts / $cid additions.)

Yes, hooks can use anything provided as by context and the handler doesn't know what they used. So everything in context needs to be considered when addressing caching, even if its not used by the handler. Things derived from context don't matter, they should be handled by caching context, and its up to hook implementations to add cacheable metadata themselves if they do anything more exotic.

But none of this matters in this issue. We already have a context. And we have caching. And caching ignores context, leading to false postives which are a bad thing, and so we should not use our current caching if we have context because no caching is much better than bad caching. And we should do this now, because its broken now and easy to fix, whereas making cool ways to cache context for edge cases is hard.

Makes sense re existing installs.

What's tricky with regard to 📌 Replace HostEntity::isEnabledForRegistration Active the behabior we give to the new isRegistrationValidForHost() we are introducing. I'm not fond of the idea we bake into it the legacy idea that any existing registration is invalid for the host if the host is not longer available in the sense defined by isAvailableForRegistration().

But if we don't keep the legacy behavior in isRegistrationValidForHost(), then we have to deliberately implement the legacy behavior elsewhere in addition alongside calls to the new isRegistrationValidForHost().

Here's a proposal that gives us maybe the best of both worlds:

In 📌 Replace HostEntity::isEnabledForRegistration Active :
(1) isRegistrationValidForHost() only checks isAvailableForRegistration() if the registration is new or changes the number of spaces. It doesn't have the legacy behavior.
(2) The logic in RegistrationAccessHandler added in 🐛 Access & validation mismatch creating/editing a registration on a disabled host Active for 'update' is changed to consider isAvailableForRegistration(), keeping current behavior that blocks access for non-admins to edit registrations on closed hosts.
(3) The RegistrationConstraintValidator is changed to consider only isRegistrationValidForHost(). This looks like a behavior change for ordinary users, but actually its not because they can't access the registration edit form to edit it anyway if the host is closed. This will have the consequence that admins are able to edit registration information (but not spaces etc.) on closed hosts. So this is a behavior change, but probably beneficial and only for admins.

In this issue:
(4) We add a setting that give site builders control over whether RegistrationAccessHandler should or should not block access to edit depending on isAvailableForRegistration(). I suggest a global setting to keep it simple, as I think sites needs here are so various that I don't have a lot of confidence its that widely useful for more than BC.

This is a bug. And one with security implications. So in this issue probably we should fix it in the quickest, simplest way we can.

Creating a good modern-Drupal caching system for context is the subject of 📌 Use a memory cache in EntityAccessControlHandler Active . I'd be grateful if you could share the idea of using VariationCache there.

What things are passed in $context anyway?

We don't know, by design. The API is intentionally agnostic. That's a part of the challenge.

jonathanshaw → created an issue.

Cacheability can only accumulate, even when removing a violation, so one object at the violation list level should suffice

I agree that (even if it might not be absolutely cache optimal) a single object at the list level should suffice. And the simplciity of that is a compelling virtue.

Having two operations differ by a single letter is likely to create confusion

Agreed. This got me thinking a bit more about operations.

And touches upon an inconsistency in our API I realised last night.:
- On the RegistrationHostAccessControlHandler we check $host->access('administer registrations', ... when we want to consider administer permissions when access checking operations like 'view registrations' etc. So host access hook fires twice, once with 'view registrations' and once with 'administer registrations'.
- But in RegistrationAccessControlHandler, in very similar circumstances, we check the administer permissions directly when evaluating 'view', instead of checking $registration->('administer',...
It's right and important that in the RegistrationHostAccessControlHandler we sometimes check $host->access('manage', which does cause a second firing of the access hook - this is an important point of extensibility. But the consistency with how we're treating administering doesn't seem quite right.

Sitting back, considering all this, and reviewing the access docs I wrote yesterday at https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... → I have these thoughts:
- Operations should be 1:1 with the things users actually do.
- The 'manage registrations' host operation should be removed. It's not a real operation, just a consequence of being a bit literal when creating these operations based on existing permissions. Access checking for the /{host}/registrations route should depend on the user having access to either the 'view registrations' or 'manage settings' or 'manage broadcast' host operations.
- We should not create an 'administer registration' operation, and we should not check the 'administer registrations' operation inside other host operations. We can check these permissions using a helper method, but not an operation.

What if the new permissions were per bundle, this would provide some separation, and has the nice side effect that they could become drop-in replacements for the soon to be retired "own" permissions.
Downside is loss of symmetry with the existing host permissions which are not per bundle.

I suggest not having these per bundle. I think having permissions tied to the host, and generic bundle permissions, provides a useful kind of orthogonality, and altogether I imagine it is more than sufficient for 99% of use cases. I think registration core is very generous in OOTB the permissions it provides; if starting from scratch I'm not sure we should be as generous as we are.

if there is a migration path for the "own" permissions.

If a user has the own permission for all registration types, we can give them the host permission. Otherwise, we have to block the module update using hook_requirements and require the sitebuilder to remove the own permissions and consider what permissions to give in light of their site's needs.

Patch is irrelevant.

The diffs look a little messy, partly because I found some semi-related things that needed tweaking, but mostly because some of the logic needed refactoring. But actually the changes here are very simple.

I suggest checking https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... → first, just in case seeing everything laid out like that raises any bigger questions.

This is a blocker (not sure if soft or hard) for contrib in 📌 Add 'create * host permissions' Postponed

jonathanshaw → created an issue.

I started work on some docs on what this should eventually look like when the remaining issues are addressed:
https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... →

Minor tweaks

First draft

jonathanshaw → created an issue.

Let's handle 'create host ... registration' permissions seperately.

Postponed on ✨ Create additional host permissions Active as that provides the new permissions that should be used instead.

Re ManageRegistrationsAccessCheck

"Do we need a couple new examples now - for administer own conference registration settings - with host update TRUE and FALSE - and expected being TRUE and FALSE respectively."

I don't think it's as simple as that. We would need to add a new scenario if I understand the existing data provider logic correctly. And I'm not sure that's worth doing given I've opened 🐛 Remove "administer own $type_id registration settings" & "manage own $type_id registration" Active as a follow up.

jonathanshaw → created an issue.

That all sounds good. I think we should have the code be required, but that should be easy enough to accomodate; and consider whether or not to allow duplicate codes.

Two more things I forgot:

(1) I think the metadata needs to be on the violation, so if you remove (or modify) a violation you naturally remove or modify the associated violation.

(2) However, we should also allow for circumstances similar to AccessResult::neutral(), i.e. cases where there is no error but there could have been, so cacheability metadata needs to be added so the result is revisited if the circumstances change so there might then be an error.

We could have something like an isActive() method on our violation object that defaults to false. Only active violations cause the result sets to be invalid, but non-active violations do have their metadata gathered in. However, that messes with the semantics of violation lists in nasty ways.

I think better is to either
(a) abandon violation lists altogether;
(b) allow them to hold a seperate set of almost-violations in addition to the actual violations; or
(c) allow for metadata to be added explicitly to the violations list for these rare cases only, so that when you get the metadata from the list you get the metadata explicitly added to the list from possible but not actual violations, combined with the metadata from each actual violation.

You're right

I think we should do this in this issue, before or after ✨ Create additional host permissions Active :

We should change the functioning of "administer own $type_id registration" so that it only addresses administering registrations the user is the registrant of; this can be done in a fully BC way by giving anyone who has it the "manage own $type_id registration" permission as well.

And then we should create a new issue to discuss (and maybe implement but only after ✨ Create additional host permissions Active ) this:

We then need to get rid of "administer own $type_id registration settings" and "manage own $type_id registration". This is impossible to do in a fully BC way. Perhaps the best solution is to use hook_requirements to block upgrading the module if those permissions are still in use, forcing sitebuilders to take action.

So there's 3 separable issues here. We could combine, but I tend to assume seperating is better if possible.

Thanks for picking this up! It's soft blocking ✨ Allow admins to change host entity for existing registration Active so I'm very keen on it.

One thing I didn't elaborate on in the IS but I do think is worth doing is the "shorter message" and "longer message" thing. Our current $errors often have lengthy verbose messages which are designed to work when shown to the user as validation messages when saving a registration.

These don't work very well when showing a list of hosts are trying to indicate which are not available for registration. In this case, you want simple 2-3 word explanations that make minimal assumptions. This came up for me in registratation_change_host when showing which hosts were and were not available to change to. But it could also come up in something like commerce_registration when trying to indicate which variations are not registerable. Yes, you can hide the host, but sometimes it's confusing to users to hide options that previously existed and sometimes it's better to "gray them out" and give a reason why they're no longer available. Because there could be multiple reasons, and we know nothing about the context, it's good to keep these very short and generic like "No spaces remaining.", "Already registered.", "Deadline passed.".

The short message could be the default required message, the longer one optional and shown primarily when saving a registration.

$metadata = new BubbleableMetadata();
$violations = $host_entity->isAvailableForRegistration($spaces, $registration, TRUE /* $return_as_object */, $metadata);
if ($violations->count() > 0) {
  // process violations and apply cacheable metadata to render array
  return $form;
}

This uses the "return as object" pattern from AccessResults, and the BubbleableMetadata pattern from Token::replace.

I like using the violations interface since EntityConstraintViolationListInterface provides the ability to filter on error types etc.

Using the "return as object" pattern sounds like a great idea.

I'm not sure we need to pass in the metadata object in this way, rather than get the metadata from the returned object. Unless you were thinking plan to return simply EntityConstraintViolationList objects.

I agree it's good to have both an "set of items" object and an "item" object.

I doubt that using simply EntityConstraintViolationList alone for the set would work. Yes, it allows filtering by field. But some of our violations aren't associated with a particular field. Drupal's violations lists don't really expect to be altered using events/hooks, so they have no support for items to have ids or some other identifying codes, and to do has/get/set by id. But this is important to us. (we currently use the $errors array keys for this).

So even if you use EntityConstraintViolationList as a base, I think you will be extending it. In which case you can make it implement a cacheable interface as well and avoid the need for the seperate cacheable metadata object.

I like the naming "isAvailable" better than "isEnabled" since "is enabled for registration"

Agreed.

Because "allow multiple" can't be checked up front (you don't always know the registrant), the logic to check that will need to stay in the RegistrationConstraintValidator.

I still believe it's good to distinguish between
$host_entity->isAvailableForRegistration($spaces, TRUE /* $return_as_object */,);
and something like
$host_entity->isRegistrationValidForHost($registration);

The point of the isRegistrationValidForHost() is that you have a registration - existing or unsaved - and so you know the registrant, and so can call isUserRegistered(), check allow multiple, etc. But it doesn't consider aspects of registration validity that are unrelated to the host, like custom fields, etc., so is simpler than calling $registration-isValid().

Whereas in isAvailableForRegistration() you don't know anything about the registration, it's more abstract.

I understand that theres no obvious compelling need to split these 2 apart, but I think it decomposes our complex logic here into 3 neat layers that build on each other:
- isAvailableForRegistration() considers only the host
- isRegistrationValidForHost() considers isAvailableForRegistration() and other aspects of the registration in the context of the host
- $registration->validate() considers isRegistrationValidForHost() and anything else about the registration

For example, in registration_change_host I only want to show hosts to which a registration can be validly changed. But because changing the host means changing the registration type , required fields could be null on the (temporary, unsaved, hypothetical) registration, and so $registration->validate() will likely have violations. But actually I only care about the host, so I want $host->isRegistrationValidForHost($registration) alone in order to assess whether this host is available to change to.

I'm excited that you will be working on this! I will make time to swiftly discuss or review anything you like.

In 📌 Add initial support for SetupIntents Active tomtech added support for customers adding payment methods using setup intents.

MOTO has complex issues; if someone wants to add support for it, perhaps best to open a new issue advocating for that in general.

As a support request, this issue seems to have run its course.

Sounds like it's not an issue with this module

Added release note snippet to IS

We already have an issue for something like this: #2602368: Allow objects that implement RenderableInterface to used interchangeably with render arrays. →

And it can be achieved in a fairly BC way.

Looks right to me.

I'm not sure if #214 was ever dealt with

@gonzalo no idea I'm afraid

The change record mentions a widget setting. But the latest MR seems to follow #26 and do this automatically without a setting.

Also there's a code fix requested on the MR in #48.

I've seen something similar ... there was a problem and then it dissappeared.

Under the hood, entity_clone is not great. It needs a lot of refactoring, which isn't happening. However, at least part of the cause is that this is a really complex problem space, especially when you consider entities referenced by the entity being cloned - should they be cloned too?

Of most concern is #270 saying that the conversation in #237/#247/#259 is unresolved. I am not sure about that. I think that alexpott answered that in #259. Did I miss something?

In #235 @alexpott makes a proposal to use a different approach. In #237 @berdir says it won't work, which @laurii agrees with in #240. But alexpott in #247 disagrees with #237. @laurii countergargues in #257, and @alexpott refutes in #259.

I don't think @alexpott's suggestion from #237 has been implemented, but he's not given up arguing for it either.

On reflection, I think this will be better with 2 helper methods, one for access and one for createAccess. The createAccess one will get enhanced in a follow up issue to better handle $context.

Also, Gitlab MRs are preferred - maybe even required - rather than patches now.

Great!

1. +++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php
   @@ -14,11 +15,11 @@
   +   * @var \Drupal\Core\Cache\MemoryCache\MemoryCache
   
   @@ -53,12 +54,13 @@ class EntityAccessControlHandler extends EntityHandlerBase implements EntityAcce
   +    $this->memoryCache = new MemoryCache();
   

   I think we need a service for this. See https://www.drupal.org/node/3409455 → for the correct pattern. Without that the cache won't get properly invalidated when dependencies are updated.

   The same service is shared between access handlers, which will make resetCache unecessarily aggressive but that doesn't seem like a big problem.

2. +++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php
   @@ -76,15 +78,13 @@ public function access(EntityInterface $entity, $operation, ?AccountInterface $a
          $cid .= ':' . $entity->getRevisionId();
   

   I'm not very comfortable with this combination of creating part of the cache key here and part in the helper method. I think it might be best to pass the entity to the helper method and build the whole cache key there, with conditional logic for 'create' bundle where necessary.

3. +++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php
   @@ -76,15 +78,13 @@ public function access(EntityInterface $entity, $operation, ?AccountInterface $a
   -      // It is not possible to delete or revert the default revision.
   

   Looks like an accidental removal.

4. +++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php
   @@ -221,17 +223,36 @@ protected function setCache($access, $cid, $operation, $langcode, AccountInterfa
   +  protected function generateCacheKey($cid, $operation, $langcode, AccountInterface $account) {
   

   Please pass the full $context array to generaateCacheKey(). This will help other issues that want to use that in the cache key.

Fixed

Even more signficantly, addressing this surfaced a significant bug in RegistrationOverrideChecker that prevented anyone from overriding the maximum spaces constraint.

Answered

It's kind of an annoying issue, but I think what I'm doing here is right. Tests that have an implict dependency on the hidden mystic powers of the superuser are easy to get wrong, it's better to be very explicit about the user and their permissions. The proof of this is in modules/registration_waitlist/tests/src/Kernel/RegistrationWaitListStateTransitionAccessTest.php which actually wasn't testing what it looked like it was testing, because the superuser was making the test too easy to pass.

Unfortunately extending EntityAccessControlHandler is a breach of the interface:

  37     Parameter #1 $host_entity (Drupal\registration\HostEntityInterface)      
         of method                                                                
         Drupal\registration\RegistrationHostAccessControlHandler::access() is    
         not contravariant with parameter #1 $entity                              
         (Drupal\Core\Entity\EntityInterface) of method                           
         Drupal\Core\Entity\EntityAccessControlHandler::access().                 
  105    Parameter #1 $host_entity (Drupal\registration\HostEntityInterface)      
         of method                                                                
         Drupal\registration\RegistrationHostAccessControlHandler::checkAccess()  
         is not contravariant with parameter #1 $entity                           
         (Drupal\Core\Entity\EntityInterface) of method                           
         Drupal\Core\Entity\EntityAccessControlHandler::checkAccess(). 

As discussed in [# https://www.drupal.org/project/drupal/issues/2886800#comment-15815518] 🐛 EntityAccessControlHandler::createAccess() and EntityAccessControlHandler::access() return false positive cache hits because it ignores context Needs work comment 58, this issue is specifically about context. The issue of ignored cacheable metadata in ::access() and ::createAccess() is the subject of 📌 Use a memory cache in EntityAccessControlHandler Active .

#46 is wrong to say "this affects access() as well as createAccess()." access() does not use $context. It's true that cacheability metadata of hooks and handler results in both access() and createAccess() is not considered. This is a bug, and the correct solution is a memory cache. But it's a different bug to this issue, and solving it does not necessarily solve the context bug discussed in this issue because context may contain items that are not cacheable dependencies. I created 📌 Use a memory cache in EntityAccessControlHandler Active to address this other bug.

The MR is based on #33/36, which @hctom in #37 and @claudiu.cristea in #44 supported. We cease to use the cache if a context is provided. As discussed in #36/44 this may have a performance impact on some custom code, but that's better than the present situation of a bug with possible security implications. As @hctom said in #37:

Everything is better than getting wrongly cached results that might even result in security issues!

Improving performance by caching even if context is present is a task for the follow-up: ✨ EntityAccessControlHandler::createAccess() should use whole context array when caching Active .

jonathanshaw → created an issue.

RegistrationAccessTest creates a user (and doesn't use it) in setUp() in order to avoid this problem, so that this unused user is the superuser.

Finally I think this is ready.

jonathanshaw → created an issue.

I've addressed the impact of [##3464137], but we need test coverage for hook_registration_host_access().

Answered on the MR, the code is all good AFAICS.

This will need updating if ✨ Make it possible to customise who can administer registrations Active is committed first, or vice versa.

jonathanshaw → created an issue.

jonathanshaw → created an issue.

I finally got the deprecation tests working.

Addressed feedback.

If 📌 Add a host access handler Needs review is committed first this would need updating, or vice versa.

I am having a lot of trouble getting the deprecation test to work on both D10 and D11.

It's true that core, and where I've found in contrib, don't use 'administer' as an operation name. But I don't understand the benefit of your proposed solution, so I will try to set out here clearly how I'm seeing it and hope you can see what I'm not seeing.

Let's not hardcode permissions
It seems to me that hardcoding permission names when doing access checks in forms, route controllers, validators etc is a bit of an anti-pattern in an ecosystem contrib module like registration, as it makes it hard for developers to customise the access. So we should do something to fix the places I identified here where administer permission are currently hard-coded.

How to not hardcode permissions
We have to allow code that wants to check access to call out to some centralised place, which then checks certain permissions by default but also allows for customisation. The normal drupal pattern is $entity->access($op) backed up by the access handler, so the access handler receives the $entity and the $op. The operation is kind of like a simplifed explanation to the access handler about what access is being sought.

Using a service instead of the handler>/strong>
As you suggest we could also use a service for our centralised extensible access logic. As you point out we would still have to pass it the entity and something like an operation, a simplifed explanation about what access is being sought:

call the new service passing "administer" as the requested access. ... The access levels that could be passed to the service ... perhaps "administer", "manage", "edit", "view" and "delete".

I don't understand the advantage of using this design pattern in addition to the normal access control handler. It seems to have basically the same inputs and basically the same purpose. The added complication/confusion would seem to outweight any benefit.

Could we just use the access handler with an operation
Core has many operations: view, view label, update, delete, revert, delete revision, edit, approve, download, use, disable, view all revisions, revert revision, publish.
So adding an additional operation of our own to describe a specific access scenario seems like a standard Drupal pattern.

What operation or operations?
I agree administer may be a bad choice. With permissions it usually means "can do absolutely anything", it has a kind of catch-all quality, whereas maybe operations are intrinsically specific.

The 3 places where we hardcode administer permissions that we are trying to fix have a lot in common: they are all places where some normal business-logic constraint on what can be done with a registration is being overridden. They are:
- the validator
- the overrides checker
- whether or not to show the status field on the registration form

I think probably these have enough in common to be treated as a single operation.
Possible names would include: 'override', 'bypass constraints' or 'manage'.

My conclusion
Add an additional operation to the existing access control handler, so we don't have hard-coded administer permissions, but name it something better than 'administer'.

I think I'd need to be some kind of maintainer to help with documentation, it's not a free access wiki like the old docs system.

I have been thinking about this in the context of ✨ Establish host variations as an architectural concept Active and come to the same conclusion as you, that it would be good to allow developers to define 'scopes' within which only a single registration is possible. I think this is a natural idea for registrations that could be used in different ways in a lot of registration systems.

I would love to coordinate on a new structure for a future 3.5 or 4.x version of the module.

Great, let's do it. I suggest we discuss on a messy way in this issue, and once we understand what's needed better, we can maybe make a new roadmap or subissues.

My current thinking is that we have 3 problematic permisisons:
"administer own $type_id registration"
"administer own $type_id registration settings"
"manage own $type_id registration"

We should create 2 new permissions to replace these:
"manage host registrations"
"administer host registrations"

We should change the functioning of "administer own $type_id registration" so that it only addresses administering registrations the user is the registrant of; this can be done in a fully BC way by giving anyone who has it the "manage own $type_id registration" permission as well.

We then need to get rid of "administer own $type_id registration settings" and "manage own $type_id registration". This is impossible to do in a fully BC way. Perhaps the best solution is to use hook_requirements to block upgrading the module if those permissions are still in use, forcing sitebuilders to take action.

Perhaps we can deal with the existing structure by creating a documentation page on d.org (I have an issue for this in the issue queue)

I will have a look at it.

I think this can be committed as is. It's a simple win.

The only behavior change with the MR as it currently stands is that users with the "administer own {$entity->bundle()} registration" will become able to do certain administery kind of things on their own registrations that they currently can't. This increases the consistency in the meaning of 'administer' between the own/any contexts.

From @wimleers #2949888-24: Enhance config schema for richer default experiences → :

Allowed values / enum

Already in core — see this for ckeditor5.plugin.ckeditor5_language for example:

Choice:
- un
- all

+1 for the idea.

Things like option values would still need to be provided in the form.

Maybe that's a limitation of the current schema that could be improved down the road.

Updated the IS with new proposal now I understand this is not a bug per se.

I'm finally understanding why this is so confusing. For 'administer own $type_id registration' the description is:
"View, edit and delete own registrations of this type. Manage registrations and registration settings of this type for host entities to which a user has edit access."

And this is in fact how it works.

What's confusing is that the semantic of own is effectively different in the 2 sentences of the description: in the first sentence "View, edit and delete own registrations" it means registrations that one is the registrant for; in the second sentence it means the same as it does for "manage own $type_id registrations", registrations for host entities you can edit.

Additionally confusing is that I would naively have understand 'Manage registrations' to mean being able to do things like view, update and edit registrations. But actually all it really means "is able to view the manage registrations route'.

So the code does match the docs, but what these permissions are doing is so weird that I've found it very hard to understand or believe.

One problem with removing 'administer own $type_id registration' altogether is that we still have 'manage own $type_id registration' so the odd semantic lingers on.

N.B. the docs 'for administer own $type_id registration' say 'Manage registrations and registration settings of this type for host entities to which a user has edit access.' Currently the code allows the user to administer the settings but not the registrations.

Part of the problem is that we also have 'manage own $type_id registration' which is documented as having the same semantic as 'administer own $type_id registration'.

We could change the documentation for 'administer own $type_id registration' to match how it is actually used (i.e. own means the registrant not the host editor) as it does for view, update and delete. But then we'd still be left with 'manage own $type_id registration' dangling with its own odd semantic.

Perhaps that could be deprecated in favor of a new 'manage host registration'.

For now, changing the documentation for 'administer own $type_id registration' to match its actual behavior seems to be the simplest solution. From a security point of view this is relatively benign as the user is getting a more restricted access than the documentation used to imply they would get.

The alexpott and laurii debate from 237/247/259 is unresolved.

Exponential backoff is a best practice. I wonder if we should implement it by default rather than making it configurable. That keeps the codebase simpler and more maintainable.

We overlooked something: we didn't harden the logic in the queue to handle the possibility that the job plugin's handleDuplicateJobs() method might return null to cancel the job.

I opened a follow-up: 🐛 Handle nulled duplicate jobs Active