ProfileForm::actions() defines new delete element instead of altering it as it expects

Created on 26 April 2014, almost 11 years ago
Updated 23 March 2023, about 2 years ago

Problem/Motivation

User entitiess do not have a delete form, they have a cancel form. This implies the action one takes with a User entity is 'cancel' rather than 'delete'.

The access handler currently responds to 'delete' actions, not 'cancel' actions.

Proposed resolution

Deprecate the 'delete' access operation in favour of 'cancel'.

Remaining tasks

Decide next steps per #39

User interface changes

None

API changes

Change to User access operations.

📌 Task
Status

Needs work

Version

10.1

Component
User system 

Last updated 2 days ago

Created by

🇨🇭Switzerland berdir Switzerland

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇺🇸United States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

    See [CR TBC]'

    Does this need a change record?

  • 🇺🇸United States smustgrave

    Change looks good. Even though it's a task wonder if we could add a simple test that if we use "delete" we get the trigger_error correctly.

  • Status changed to Needs review about 2 years ago
  • 🇦🇺Australia darvanen Sydney, Australia

    Yep, good point, this also meant I discovered I hadn't set the error type, so that's done now. Test behaves properly on local so here goes nothing.

  • 🇦🇺Australia darvanen Sydney, Australia

    Oops, crafted the test patch badly, here's what it should have been.

  • 🇮🇳India Ajeet Tiwari

    uploaded patch for 10.1.x.

  • Status changed to Needs work about 2 years ago
  • 🇦🇺Australia darvanen Sydney, Australia

    So, @sun was right in #5 (of course they were). API resources make this a bit of a challenge.

    For example in \Drupal\jsonapi\Routing\Routes::getIndividualRoutesForResourceType line 308 we have

    $individual_remove_route->setRequirement('_entity_access', "entity.delete");

    which sets the route permission for *any* delete operation on an entity.

    So we're faced with a few options:

    1. Put special cases into the API systems to cater for the "cancel" entity access value, or
    2. Instead of deprecating the delete key for users, differentiate between them for API vs Forms, possibly with an additional permission, or
    3. Leave well enough alone.

    Honestly I'm inclined towards the third option.

    Hiding patch from #37 as it regressed functionality, did not apply, has no interdiff, and did not come with any explanation of what it was trying to achieve.

  • Status changed to Needs review about 2 years ago
  • 🇮🇳India Ajeet Tiwari

    Uploading patch again for 10.1.x after resolvng the errors of custom commands.

  • Status changed to Needs work about 2 years ago
  • 🇺🇸United States smustgrave

    Per #39

  • 🇺🇸United States smustgrave

    Thank you for the interest @Ajeet Tiwari but it is expected to test a patch before uploading so have to remove credit for that.

    Just FYI to help get the message out there.

    Starting March 2023, simple rerolls, rebases, or merges will no longer receive issue credit. Only rerolls that address a merge conflict will be credited, and the merge conflict that was resolved must be documented in the text of an issue comment.
    To receive credit for contributing to this issue, assist with other outstanding tasks or unaddressed feedback.
    See the issue credit guidelines for more information.

  • Status changed to Needs review about 2 years ago
  • 🇮🇳India Ajeet Tiwari

    tried to fix the CCF error and to fix the error in #34.

  • 🇮🇳India _utsavsharma

    Fixed CCF for #43.

  • Status changed to Needs work about 2 years ago
  • 🇺🇸United States smustgrave

    Hiding #43 as there is no interdiff being provided. Also removing credit as it's expected patches to be tested before uploading. #43 appears to be adding additional changes also.

    #44 carried forward those additional changes.

    Please read #39 before continuing.

    @darvanen would agree on option 3 also FWIW

Production build 0.71.5 2024