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

feat(api): PUT api to Set permissions for a upload and GET API to fetch respective permissions of groups #2275

Merged
merged 2 commits into from Sep 2, 2022

Conversation

krishna9304
Copy link
Contributor

@krishna9304 krishna9304 commented Jul 22, 2022

Signed-off-by : Krishna Mahato krishhtrishh9304@gmail.com

Description

Now,

  • There is a PUT route which is available at /api/v1/uploads/{id}/permissions that will Set permissions for a upload in a folder for different groups.
  • There is a GET route which is available at /api/v1/uploads/{id}/perm-groups that will return all the groups with their respective permissions for a upload.
    Here {id} : is the upload Id.

How to test

  • Use any API platform like postman.
  • Pull the changes from this PR.

To test PUT /api/v1/uploads/{id}/permissions

  • Provide the request body as following ----

Screenshot 2022-07-23 at 2 25 39 AM

  • You can expect a response like this

Screenshot 2022-07-23 at 2 26 11 AM

To test GET /api/v1/uploads/{id}/perm-groups

  • If your upload exists, you can expect a response like this.

Screenshot 2022-08-24 at 11 43 23 PM

PTAL : @GMishx @Shruti3004

This closes #2270 .

@krishna9304 krishna9304 changed the title feat(api): /uploads/{id}/permissions PUT api to Set permissions for a… feat(api): /uploads/{id}/permissions PUT api to Set permissions for a… Jul 23, 2022
@GMishx GMishx added the GSoC-22 Label to tag issues and pull request for GSOC 2022 activities label Jul 25, 2022
src/www/ui/api/documentation/openapi.yaml Outdated Show resolved Hide resolved
src/www/ui/api/documentation/openapi.yaml Outdated Show resolved Hide resolved
src/www/ui/api/documentation/openapi.yaml Outdated Show resolved Hide resolved
src/www/ui/api/documentation/openapi.yaml Outdated Show resolved Hide resolved
src/www/ui/api/documentation/openapi.yaml Outdated Show resolved Hide resolved
src/www/ui/api/documentation/openapi.yaml Outdated Show resolved Hide resolved
src/www/ui/api/documentation/openapi.yaml Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/UploadController.php Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/UploadController.php Outdated Show resolved Hide resolved
@GMishx
Copy link
Member

GMishx commented Aug 8, 2022

@krishna9304 apologies, I pressed the wrong button an pushed the merge commit to the branch. Can you please rebase it with current master and remove the merge commit?

src/www/ui/api/documentation/openapi.yaml Outdated Show resolved Hide resolved
src/www/ui/api/documentation/openapi.yaml Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/UploadController.php Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/UploadController.php Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/UploadController.php Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/UploadController.php Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/UploadController.php Outdated Show resolved Hide resolved
@krishna9304
Copy link
Contributor Author

@krishna9304 apologies, I pressed the wrong button an pushed the merge commit to the branch. Can you please rebase it with current master and remove the merge commit?

Sure, I'll do that!

Copy link
Member

@GMishx GMishx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks good.

Copy link
Member

@GMishx GMishx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Please check the image for required changes in response of GET repo/api/v1/uploads/{id}/perm-groups endpoint.

The PUT repo/api/v1/uploads/{id}/permissions endpoint is working as expected.

src/www/ui/api/index.php Show resolved Hide resolved
@krishna9304
Copy link
Contributor Author

image

Please check the image for required changes in response of GET repo/api/v1/uploads/{id}/perm-groups endpoint.

The PUT repo/api/v1/uploads/{id}/permissions endpoint is working as expected.

@GMishx

  • Okay, I'll remove the perm_upload_pk parameter.
  • Sending an indexed object for the groups with permissions response makes it easy to implement on the UI, as each group can be directly targeted through their groupId and perform edit or delete operation for the UI purposes. Handling this with an array will increase complexity on the UI and can result in behavioural delays (not good from UX perspective).

@krishna9304 krishna9304 changed the title feat(api): /uploads/{id}/permissions PUT api to Set permissions for a… feat(api): PUT api to Set permissions for a upload and GET API to fetch respective permissions of groups Aug 14, 2022
@GMishx
Copy link
Member

GMishx commented Aug 24, 2022

@GMishx

* Okay, I'll remove the `perm_upload_pk` parameter.

* Sending an indexed object for the groups with permissions response makes it easy to implement on the UI, as each group can be directly targeted through their `groupId` and perform edit or delete operation for the UI purposes. Handling this with an array will increase complexity on the UI and can result in behavioural delays (not good from UX perspective).

@krishna9304

Please keep in mind the REST API is not designed solely for the purpose of UI implementation. It can be consumed by others as well. Using indexed array can cause issues with some programming languages. However, simple array can be iterated over and filtered based on inner group_pk.

Moreover, I am not sure the array will be so big for a single upload that it will cause significant computational delay if all the elements have to be iterated over.

@krishna9304
Copy link
Contributor Author

@GMishx

* Okay, I'll remove the `perm_upload_pk` parameter.

* Sending an indexed object for the groups with permissions response makes it easy to implement on the UI, as each group can be directly targeted through their `groupId` and perform edit or delete operation for the UI purposes. Handling this with an array will increase complexity on the UI and can result in behavioural delays (not good from UX perspective).

@krishna9304

Please keep in mind the REST API is not designed solely for the purpose of UI implementation. It can be consumed by others as well. Using indexed array can cause issues with some programming languages. However, simple array can be iterated over and filtered based on inner group_pk.

Moreover, I am not sure the array will be so big for a single upload that it will cause significant computational delay if all the elements have to be iterated over.

Sure then, I'm changing the response to be an array. @GMishx

@GMishx
Copy link
Member

GMishx commented Aug 25, 2022

@krishna9304 some changes are required in open api docs

diff --git a/src/www/ui/api/documentation/openapi.yaml b/src/www/ui/api/documentation/openapi.yaml
index 82e1a7e5d..7d3598c43 100644
--- a/src/www/ui/api/documentation/openapi.yaml
+++ b/src/www/ui/api/documentation/openapi.yaml
@@ -677,7 +677,7 @@ paths:
         Set permissions for a upload in a folder for different groups
       requestBody:
         content:
-          application/x-www-form-urlencoded:
+          application/json:
             schema:
               type: object
               properties:
@@ -2351,18 +2351,17 @@ components:
           type: string
           description: Public permission for the upload
         permGroups:
-          type: object
+          type: array
           description: Indexed Object of GroupIds with their respective permissions for a upload
-          properties:
-            groupId:
-              type: object
-              properties:
-                perm:
-                  type: string
-                group_pk:
-                  type: string
-                group_name:
-                  type: string
+          items:
+            type: object
+            properties:
+              perm:
+                type: string
+              group_pk:
+                type: string
+              group_name:
+                type: string
     UrlUpload:
       description: To create an upload from a URL
       type: object

@krishna9304
Copy link
Contributor Author

@krishna9304 some changes are required in open api docs

diff --git a/src/www/ui/api/documentation/openapi.yaml b/src/www/ui/api/documentation/openapi.yaml
index 82e1a7e5d..7d3598c43 100644
--- a/src/www/ui/api/documentation/openapi.yaml
+++ b/src/www/ui/api/documentation/openapi.yaml
@@ -677,7 +677,7 @@ paths:
         Set permissions for a upload in a folder for different groups
       requestBody:
         content:
-          application/x-www-form-urlencoded:
+          application/json:
             schema:
               type: object
               properties:
@@ -2351,18 +2351,17 @@ components:
           type: string
           description: Public permission for the upload
         permGroups:
-          type: object
+          type: array
           description: Indexed Object of GroupIds with their respective permissions for a upload
-          properties:
-            groupId:
-              type: object
-              properties:
-                perm:
-                  type: string
-                group_pk:
-                  type: string
-                group_name:
-                  type: string
+          items:
+            type: object
+            properties:
+              perm:
+                type: string
+              group_pk:
+                type: string
+              group_name:
+                type: string
     UrlUpload:
       description: To create an upload from a URL
       type: object

yes, may be I missed that, I'm fixing it right away.

… upload in a folder for different groups

/uploads/{id}/perm-groups GET api to get all the groups with their respective permissions for a upload

Signed-off-by: Krishna Mahato<krishhtrishh9304@gmail.com>
@krishna9304
Copy link
Contributor Author

@GMishx Fixed api docs.

Signed-off-by: Gaurav Mishra <mishra.gaurav@siemens.com>
Copy link
Member

@GMishx GMishx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks good.
Tested, working as expected.

@GMishx GMishx added ready and removed needs test labels Sep 2, 2022
@GMishx GMishx merged commit 31de1a0 into fossology:master Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSoC-22 Label to tag issues and pull request for GSOC 2022 activities ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a POST route to add or edit permissions for different uploads in a folder
3 participants