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

[MNT-24127] Added Endpoint To Calculate Folder Size #213

Merged
merged 7 commits into from
Jun 24, 2024

Conversation

mohit-singh4
Copy link
Contributor

@mohit-singh4 mohit-singh4 commented Jun 18, 2024

@suneet-gupta
Copy link
Contributor

suneet-gupta commented Jun 18, 2024

It's recommended to add Jira ticket number in square bracket in title at start. This will link the PR to the corresponding Jira ticket. Also It's a good practise to add jira ttcket link in the description.

Copy link
Contributor

@suneet-gupta suneet-gupta left a comment

Choose a reason for hiding this comment

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

Update the title and description

@mohit-singh4 mohit-singh4 changed the title feature/MNT-24127-Added endpoint to calculate size [MNT-24127] Added Endpoint To Calculate Folder Size Jun 19, 2024
@@ -2484,6 +2491,92 @@ paths:
description: Unexpected error
schema:
$ref: '#/definitions/Error'
'/nodes/{nodeId}/calculateSize':
post:
x-alfresco-since: "5.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we supporting this API from ACS 5.2 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I have removed the same.

schema:
$ref: '#/definitions/Error'
get:
x-alfresco-since: "5.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above. Are we supporting this API from ACS 5.2 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I have removed the same.

post:
tags:
- nodes
summary: Calculate a folder size
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we supporting the API to calculate the size of a file. If yes, then we shd mention in the description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are only calculating the size of a folder not file.

default:
description: Unexpected error
schema:
$ref: '#/definitions/Error'
Copy link
Contributor

@suneet-gupta suneet-gupta Jun 20, 2024

Choose a reason for hiding this comment

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

What error code are we returning in case its a valid node id but this is a retention-schedule node id i.e. invalid node type

Copy link
Contributor Author

@mohit-singh4 mohit-singh4 Jun 21, 2024

Choose a reason for hiding this comment

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

We are returning 404- nodeId does not exist, as we are only considering folder type node.

Copy link
Contributor

@suneet-gupta suneet-gupta Jun 21, 2024

Choose a reason for hiding this comment

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

But here node id exist in the system but it has different node type. I think we should have a different message and may be status code as well. Please check other endpoints.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated with 400 status code. with description

Copy link
Contributor

Choose a reason for hiding this comment

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

Plz once validate the message with other endpoints in the swagger so that we are having similar standards. Rest looks good to me.

calculatedAtTime:
type: string
status:
type: string
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a good practice to add description for all the fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

type: object
properties:
executionId:
type: string
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above. Add the desctiption for the field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

type: string
description: Provides the time when the calculating folder size will be completed.
status:
type: string
Copy link
Contributor

@suneet-gupta suneet-gupta Jun 21, 2024

Choose a reason for hiding this comment

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

I think we should have "number of files" field in the response. This was earlier part of RFC confluence page as well and is in sync with the requirement from Product Manager.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the same.
Done

@suneet-gupta suneet-gupta requested a review from SathishK-T June 24, 2024 06:46
Copy link
Contributor

@suneet-gupta suneet-gupta left a comment

Choose a reason for hiding this comment

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

LGTM

@mohit-singh4 mohit-singh4 merged commit d0c1ddb into master Jun 24, 2024
11 checks passed
@mohit-singh4 mohit-singh4 deleted the feature/MNT-24127-AddedEndpointToCalculateSize branch June 24, 2024 06:55
mohit-singh4 added a commit that referenced this pull request Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants