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): Add max page size #377

Merged
merged 12 commits into from
Aug 1, 2024
Merged

Conversation

DhairyaMajmudar
Copy link
Contributor

Description

Adding max page size to 30

Fixes #160

Dependencies

N/A

Future Improvements

N/A

Mentions

@rajdip-b

Screenshots of relevant screens

N/A

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

Copy link
Contributor

Failed to generate code suggestions for PR

@rajdip-b rajdip-b added the foss hack Clustering all the curated issues for Foss Hack 2024 label Jul 27, 2024
@rajdip-b rajdip-b changed the title fix: Adding max page size feat(api): Add max page size Jul 27, 2024
@@ -763,7 +763,7 @@ export class WorkspaceService {
//get all workspaces of user for page with limit
const items = await this.prisma.workspace.findMany({
skip: page * limit,
take: Math.min(limit, 30),
take: Number(limit),
Copy link
Member

Choose a reason for hiding this comment

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

This won't be necessary. Just limit will do.

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 👍🏻

Copy link
Member

Choose a reason for hiding this comment

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

You would need to pass limitMaxItemsPerPage(limit) in here :(

@rajdip-b
Copy link
Member

image

You would also need to send in the max limit in here. I would say, create a util function name limit-max-items-per-page.ts and call that function across all the limits. This way, later on when we need to change the capacity, we only change the function and not all the code.

Copy link
Member

@rajdip-b rajdip-b left a comment

Choose a reason for hiding this comment

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

For all the files I have the same comments:

  • Get rid of Number() function
  • Replace limit: Number(limit), with limit: limitMaxItemsPerPage(limit), in every paginate function call

@DhairyaMajmudar
Copy link
Contributor Author

For all the files I have the same comments:

* Get rid of `Number()` function

* Replace `limit: Number(limit),` with `limit: limitMaxItemsPerPage(limit),` in every `paginate` function call

sure, and I've to do any changes with page: Number(page), or that's correct ?

Copy link
Member

@rajdip-b rajdip-b left a comment

Choose a reason for hiding this comment

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

Replace all occurences of page: Number(page) with page,

apps/api/src/environment/service/environment.service.ts Outdated Show resolved Hide resolved
apps/api/src/secret/service/secret.service.ts Outdated Show resolved Hide resolved
apps/api/src/workspace/service/workspace.service.ts Outdated Show resolved Hide resolved
Copy link
Member

@rajdip-b rajdip-b left a comment

Choose a reason for hiding this comment

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

instead of using page: page, just use page,

@@ -763,7 +763,7 @@ export class WorkspaceService {
//get all workspaces of user for page with limit
const items = await this.prisma.workspace.findMany({
skip: page * limit,
take: Math.min(limit, 30),
take: Number(limit),
Copy link
Member

Choose a reason for hiding this comment

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

You would need to pass limitMaxItemsPerPage(limit) in here :(

@rajdip-b rajdip-b merged commit db5bd4f into keyshade-xyz:develop Aug 1, 2024
4 checks passed
rajdip-b pushed a commit that referenced this pull request Sep 5, 2024
rajdip-b pushed a commit that referenced this pull request Sep 5, 2024
## [2.4.0](v2.3.0...v2.4.0) (2024-09-05)

### 🚀 Features

* **api-client:** Create controller for Event module ([#399](#399)) ([122df35](122df35))
* **api-client:** Create controller for Integration module ([#397](#397)) ([697d38b](697d38b))
* **api-client:** Create controller for Project module ([#370](#370)) ([fa25866](fa25866))
* **api-client:** Create controller for Secret module ([#396](#396)) ([7e929c0](7e929c0))
* **api-client:** Create controller for Variable module ([#395](#395)) ([3e114d9](3e114d9))
* **api:** Add global search in workspace ([c49962b](c49962b))
* **api:** Add max page size ([#377](#377)) ([ed18eb0](ed18eb0))
* **cli:** Add functionality to operate on Environments ([#324](#324)) ([4c6f3f8](4c6f3f8))
* **cli:** Quit on decryption failure ([#381](#381)) ([1349d15](1349d15))

### 🐛 Bug Fixes

* **api-client:** Fixed broken export ([096df2c](096df2c))
* **api:** Add NotFound exception on passing an invalid roleId while inviting user in workspace ([#408](#408)) ([ab441db](ab441db))
* **cli:** Fixed missing module ([f7a091f](f7a091f))
* **platform:**  Build failure in platform ([#385](#385)) ([90dcb2c](90dcb2c))

### 🔧 Miscellaneous Chores

* Add api client build script and updated CI ([da0e27a](da0e27a))
* **api:** Reorganized import using path alias ([d5befd1](d5befd1))
* **ci:** Update CLI CI name ([8f4c456](8f4c456))
* **cli:** Add Zod validation to parseInput function ([#362](#362)) ([34e6c39](34e6c39))
* Fixed api client tests and rearranged controllers ([1307604](1307604))
* Housekeeping ([c5f1330](c5f1330))
* **platform:** Added strict null check ([072254f](072254f))
* **web:** Added strict null check ([7e12b47](7e12b47))

### 🔨 Code Refactoring

* **api:** Update logic for forking projects ([#398](#398)) ([4cf3838](4cf3838))
Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
## [2.4.0](keyshade-xyz/keyshade@v2.3.0...v2.4.0) (2024-09-05)

### 🚀 Features

* **api-client:** Create controller for Event module ([keyshade-xyz#399](keyshade-xyz#399)) ([122df35](keyshade-xyz@122df35))
* **api-client:** Create controller for Integration module ([keyshade-xyz#397](keyshade-xyz#397)) ([697d38b](keyshade-xyz@697d38b))
* **api-client:** Create controller for Project module ([keyshade-xyz#370](keyshade-xyz#370)) ([fa25866](keyshade-xyz@fa25866))
* **api-client:** Create controller for Secret module ([keyshade-xyz#396](keyshade-xyz#396)) ([7e929c0](keyshade-xyz@7e929c0))
* **api-client:** Create controller for Variable module ([keyshade-xyz#395](keyshade-xyz#395)) ([3e114d9](keyshade-xyz@3e114d9))
* **api:** Add global search in workspace ([c49962b](keyshade-xyz@c49962b))
* **api:** Add max page size ([keyshade-xyz#377](keyshade-xyz#377)) ([ed18eb0](keyshade-xyz@ed18eb0))
* **cli:** Add functionality to operate on Environments ([keyshade-xyz#324](keyshade-xyz#324)) ([4c6f3f8](keyshade-xyz@4c6f3f8))
* **cli:** Quit on decryption failure ([keyshade-xyz#381](keyshade-xyz#381)) ([1349d15](keyshade-xyz@1349d15))

### 🐛 Bug Fixes

* **api-client:** Fixed broken export ([096df2c](keyshade-xyz@096df2c))
* **api:** Add NotFound exception on passing an invalid roleId while inviting user in workspace ([keyshade-xyz#408](keyshade-xyz#408)) ([ab441db](keyshade-xyz@ab441db))
* **cli:** Fixed missing module ([f7a091f](keyshade-xyz@f7a091f))
* **platform:**  Build failure in platform ([keyshade-xyz#385](keyshade-xyz#385)) ([90dcb2c](keyshade-xyz@90dcb2c))

### 🔧 Miscellaneous Chores

* Add api client build script and updated CI ([da0e27a](keyshade-xyz@da0e27a))
* **api:** Reorganized import using path alias ([d5befd1](keyshade-xyz@d5befd1))
* **ci:** Update CLI CI name ([8f4c456](keyshade-xyz@8f4c456))
* **cli:** Add Zod validation to parseInput function ([keyshade-xyz#362](keyshade-xyz#362)) ([34e6c39](keyshade-xyz@34e6c39))
* Fixed api client tests and rearranged controllers ([1307604](keyshade-xyz@1307604))
* Housekeeping ([c5f1330](keyshade-xyz@c5f1330))
* **platform:** Added strict null check ([072254f](keyshade-xyz@072254f))
* **web:** Added strict null check ([7e12b47](keyshade-xyz@7e12b47))

### 🔨 Code Refactoring

* **api:** Update logic for forking projects ([keyshade-xyz#398](keyshade-xyz#398)) ([4cf3838](keyshade-xyz@4cf3838))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
foss hack Clustering all the curated issues for Foss Hack 2024
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add maximum page size in fetch endpoints
2 participants