Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Content+Permission REST improvements #22993

Closed
wezell opened this issue Sep 14, 2022 · 5 comments · Fixed by #23272
Closed

Content+Permission REST improvements #22993

wezell opened this issue Sep 14, 2022 · 5 comments · Fixed by #23272

Comments

@wezell
Copy link
Contributor

wezell commented Sep 14, 2022

Make the permissions work on every action

I found some details using System Workflow and this endpoint PUT localhost:8080/api/v1/workflow/actions/fire:

  1. The endpoint only works with Publish action.
    This works:
{
    "actionName": "publish",
    "comments": "publish content",
    "contentlet": {
      "contentType":"MyType",
      "title": "Content 1",
      "identifier": "db982967e39ffaf6262f00be12679728"
    },
    "individualPermissions": {
          "EDIT_PERMISSIONS":["02ae46fa-cb67-4ed8-82d5-f1f9a5e1d744"],
          "CAN_ADD_CHILDREN": ["e37accbd-6a67-4ff4-adfe-cff423030a0b"],
          "READ": ["9ad24203-ae6a-4e5e-aa10-a8c38fd11f17"],
          "WRITE":["b9236d3a-41d4-4efd-a695-cb0f758cbf86"],
          "PUBLISH":["d8beb217-1889-40dd-99ad-fbd3cab7c426", "d3e78673-044a-4e1e-a38a-56f48cc6d5a5", "9ad24203-ae6a-4e5e-aa10-a8c38fd11f17"]
    }
}  

but the other actions, like Save doesn't work:

{
    "actionName": "save",
    "comments": "save content",
    "contentlet": {
      "contentType":"MyType",
      "title": "Content 1",
      "identifier": "db982967e39ffaf6262f00be12679728"
    },
    "individualPermissions": {
          "EDIT_PERMISSIONS":["02ae46fa-cb67-4ed8-82d5-f1f9a5e1d744"],
          "CAN_ADD_CHILDREN": ["e37accbd-6a67-4ff4-adfe-cff423030a0b"],
          "READ": ["9ad24203-ae6a-4e5e-aa10-a8c38fd11f17"],
          "WRITE":["b9236d3a-41d4-4efd-a695-cb0f758cbf86"],
          "PUBLISH":["d8beb217-1889-40dd-99ad-fbd3cab7c426", "d3e78673-044a-4e1e-a38a-56f48cc6d5a5", "9ad24203-ae6a-4e5e-aa10-a8c38fd11f17"]
    }
} 

If one permission fails to save, just write an error and continue with the rest

  1. If I type an invalid roleId, the response is 200 (OK) and all the permissions added after the invalid one are ignored:
{
    "actionName": "publish",
    "comments": "publish content",
    "contentlet": {
      "contentType":"MyType",
      "title": "Content 1",
      "identifier": "db982967e39ffaf6262f00be12679728"
    },
    "individualPermissions": {
          "EDIT_PERMISSIONS":["02ae46fa-cb67-4ed8-82d5-f1f9a5e1d744"],
          "CAN_ADD_CHILDREN": ["e37accbd-6a67-4ff4-adfe-cff423030a0b"],
          "READ": ["dfgdgdfgdfg"], --> Invalid one
          "WRITE":["b9236d3a-41d4-4efd-a695-cb0f758cbf86"], --> Ignored
          "PUBLISH":["d8beb217-1889-40dd-99ad-fbd3cab7c426", "d3e78673-044a-4e1e-a38a-56f48cc6d5a5", "9ad24203-ae6a-4e5e-aa10-a8c38fd11f17"] --> Ignored
    }
}  

When passing the roles to permission, allow roles to be specified by Role Key (which for users is a userId) OR Role ID

You should be able to pass either a role's UUID or the Role Key for any role

Follow the Null, Empty Collection, Collection rule of our other dependencies

  • if the individualPermissions is not specified (null) we should not do anything with permissions
  • if the individualPermissions is an empty map, we should wipe out all permissions
  • if the individualPermissions are specified, we replace all the permissions with the new list

Originally posted by @nollymar in #22418 (comment)

@bryanboza
Copy link
Member

We need to catch the exception in case you send an incorrect id, we are getting 500 error code instead the 404
image
The message is ok, but the error code need some work

@wezell wezell assigned wezell and jdotcms and unassigned wezell Sep 27, 2022
jdotcms added a commit that referenced this issue Sep 29, 2022
jdotcms added a commit that referenced this issue Oct 3, 2022
jdotcms added a commit that referenced this issue Oct 5, 2022
jdotcms added a commit that referenced this issue Oct 5, 2022
jdotcms added a commit that referenced this issue Oct 19, 2022
jdotcms added a commit that referenced this issue Oct 19, 2022
jdotcms added a commit that referenced this issue Oct 21, 2022
jdotcms added a commit that referenced this issue Nov 2, 2022
jdotcms added a commit that referenced this issue Nov 2, 2022
jdotcms added a commit that referenced this issue Nov 2, 2022
@jdotcms
Copy link
Contributor

jdotcms commented Nov 9, 2022

PR

jdotcms added a commit that referenced this issue Nov 15, 2022
jdotcms added a commit that referenced this issue Nov 21, 2022
jdotcms added a commit that referenced this issue Nov 21, 2022
jdotcms added a commit that referenced this issue Nov 23, 2022
jdotcms added a commit that referenced this issue Mar 10, 2023
jdotcms added a commit that referenced this issue Mar 13, 2023
jdotcms added a commit that referenced this issue Mar 13, 2023
jdotcms added a commit that referenced this issue Mar 14, 2023
jdotcms added a commit that referenced this issue Mar 14, 2023
jdotcms added a commit that referenced this issue Mar 14, 2023
nollymar pushed a commit that referenced this issue Mar 14, 2023
#23272)

* #22993 sending the PR again since the previous is having a weird behavior on test

* #22993 seems to exists a conflict since an unit test and it test have the same name

* #22993 seems to exists a conflict since an unit test and it test have the same name

* #22993 fixing an unit test issue

* #22993 fixing save draft

* #22993 adding fixes for curl test

* #22993 adding fixes for curl test

* #22993 adding fixes for curl test

* #22993 adding fixes for curl test
oidacra pushed a commit that referenced this issue Mar 16, 2023
#23272)

* #22993 sending the PR again since the previous is having a weird behavior on test

* #22993 seems to exists a conflict since an unit test and it test have the same name

* #22993 seems to exists a conflict since an unit test and it test have the same name

* #22993 fixing an unit test issue

* #22993 fixing save draft

* #22993 adding fixes for curl test

* #22993 adding fixes for curl test

* #22993 adding fixes for curl test

* #22993 adding fixes for curl test
@dsilvam dsilvam removed the Needs QA label Mar 21, 2023
@josemejias11
Copy link

josemejias11 commented Apr 13, 2023

Failed QA - Tested on 23.05_3650a5d0_SNAPSHOT // Docker // macOS 13.0 // FF v111.0

Passing the following test

  1. If I type an invalid roleId, the response is 200 (OK) and all the permissions added after the invalid one are ignored
    Screenshot 2023-04-13 at 10 55 36 AM

  2. if the individualPermissions is not specified (null) we should not do anything with permissions
    Screenshot 2023-04-13 at 10 58 59 AM

  3. if the individualPermissions is an empty map, we should wipe out all permissions
    Screenshot 2023-04-13 at 10 58 59 AM

  4. if the individualPermissions are specified, we replace all the permissions with the new list
    Screenshot 2023-04-13 at 10 59 48 AM

@bryanboza
Copy link
Member

For this issue we are ok with the specific scenarions, we will move the failing case to a new card in order to test the correct response codes in all the resource. #24627

@josemejias11
Copy link

Approved QA - Tested on 23.05_3650a5d0_SNAPSHOT // Docker // macOS 13.0 // FF v111.0

Failing test will be addressed in card #24627

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment