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.

Sign up for GitHub

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

Jump to bottom

feat: courses & instructor cursor based pagination #101

Merged
merged 24 commits into from
Feb 2, 2025
Merged

feat: courses & instructor cursor based pagination #101

merged 24 commits into from
Feb 2, 2025

Conversation

estao1
Copy link
Contributor

@estao1 estao1 commented Jan 28, 2025 •
edited
Loading

Description

Added cursor-based pagination using unique identifiers (id for courses and ucinetid for instructors) as an more efficient alternative to skip-take pagination. Includes new /coursesCursor and /instructorsCursor routes with cursor and take parameters, which returns paginated results with a nextCursor field.

Related Issue

Completes #19.

Motivation and Context

Skip-and-take pagination can become less efficient for queries where the desired items are later in the set. Cursor-based pagination helps ensure consistent query performance, especially for larger datasets.

How Has This Been Tested?

Tested changes locally on API docs and Yoga GraphiQL.

Screenshots (if appropriate):

image image image image image image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code involves a change to the database schema.
  • My code requires a change to the documentation.

estao1 added 18 commits January 13, 2025 20:29
@estao1
added cursor based pagination for courses
ea6578a
@estao1
Merge branch 'main' into 19-cursor-based-pagination
f6f3155
@estao1
feat(cursor): revised cursor-based pagination support for course quer…
eaf25bb 
…ies with nextCursor
@estao1
feat(cursor): added cursor-based GraphQL query support for courses by…
175e18f 
… cursor
@estao1
feat(cursor): revised cursor-based filtering for courses with new cou…
974364b 
…rsesCursor API route
@estao1
feat(cursor): added coursesByCursor query schema with cursor-based fi…
cd54cdd 
…ltering options, reverted skip/take query schema
@estao1
feat(cursor): added coursesByCursor resolver and coursesCursor API ro…
e454190 
…ute for cursor-based pagination
@estao1
Merge branch 'main' into 19-cursor-based-pagination
4736a7e
@estao1
feat(cursor): Revised coursesByCursor resolver to support GraphQL schema
ca41df4
@estao1
feat(cursor): Updated cursor logic to be inclusive of cursor
c0b638d
@estao1
feat(cursor): Added instructorsByCursor resolver and updated GraphQL …
553925c 
…schema for cursor-based pagination
@estao1
feat(cursor): Added instructorsByCursor route for cursor-based pagina…
6b40e5a 
…tion of instructors
@estao1
feat(cursor): Added instructorsByCursor query schema
740e6f9
@estao1
feat(cursor): Added instructors services to support cursor-based pagi…
cad6526 
…nation
@estao1
feat(cursor): Renaming and refactoring for clarity
ff0529d
@estao1
feat(cursor): Handle zero take input for nextCursor in courses and in…
488e908 
…structors services
@estao1
feat(cursor): linting and removed log statements
d4bf49a
@estao1
Merge branch 'main' into 19-cursor-based-pagination
db5a996
@estao1 estao1 temporarily deployed to staging-101 January 28, 2025 22:09 — with GitHub Actions Inactive
@laggycomputer
Copy link
Member

I get a 500 on GraphQL introspection. Can you confirm?

@estao1 estao1 temporarily deployed to staging-101 January 30, 2025 06:44 — with GitHub Actions Inactive
@estao1
Copy link
Contributor Author

estao1 commented Jan 30, 2025

Somehow forgot to commit this fix, should be good now.

@estao1 estao1 linked an issue Jan 30, 2025 that may be closed by this pull request
[Courses/Instructors] Cursor based pagination #19
Closed
laggycomputer
laggycomputer requested changes Jan 31, 2025
View reviewed changes
apps/api/src/rest/routes/courses.ts Outdated Show resolved Hide resolved
apps/api/src/rest/index.ts Show resolved Hide resolved
apps/api/src/rest/routes/courses.ts Outdated Show resolved Hide resolved
apps/api/src/schema/courses.ts Outdated Show resolved Hide resolved
apps/api/src/schema/courses.ts Outdated Show resolved Hide resolved
laggycomputer
laggycomputer requested changes Jan 31, 2025
View reviewed changes
apps/api/src/schema/courses.ts Outdated Show resolved Hide resolved
@estao1 estao1 temporarily deployed to staging-101 February 1, 2025 10:00 — with GitHub Actions Inactive
laggycomputer
laggycomputer requested changes Feb 1, 2025
View reviewed changes
apps/api/src/rest/routes/courses.ts Outdated Show resolved Hide resolved
@estao1 estao1 temporarily deployed to staging-101 February 2, 2025 00:53 — with GitHub Actions Inactive
@estao1 estao1 requested a review from laggycomputer February 2, 2025 01:01
laggycomputer
laggycomputer requested changes Feb 2, 2025
View reviewed changes
apps/api/src/schema/search.ts Outdated Show resolved Hide resolved
@estao1 estao1 temporarily deployed to staging-101 February 2, 2025 06:24 — with GitHub Actions Inactive
@laggycomputer
Copy link
Member

There's also .int; you might want to use that for validation.

andrew-wang0
andrew-wang0 approved these changes Feb 2, 2025
View reviewed changes
Copy link
Member

@andrew-wang0 andrew-wang0 left a comment

Choose a reason for hiding this comment

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

🔥

@andrew-wang0 andrew-wang0 changed the title [Courses/Instructors] Cursor based pagination feat: courses & instructor cursor based pagination Feb 2, 2025
@andrew-wang0 andrew-wang0 merged commit dc9c248 into main Feb 2, 2025
1 check passed
@andrew-wang0 andrew-wang0 deleted the 19-cursor-based-pagination branch February 2, 2025 21:12
This was referenced Feb 3, 2025
Closed
Merged
ecxyzzy pushed a commit that referenced this pull request Feb 3, 2025
@laggycomputer
hotfix(courses,instructors): correct batch route precedence (#108)
8e8608b 
## Description

Fix the issue described below, introduced in #101.

## Related Issue

Fix #107.

## Motivation and Context

we can just debug things

## How Has This Been Tested?

Tested on local deployment

## Screenshots (if appropriate):


![image](https://github.com/user-attachments/assets/15396274-8013-4fd2-aeb5-e2a63b269474)


![Screenshot_20250202_214836](https://github.com/user-attachments/assets/c84820aa-5889-416d-9b5d-6699d5394276)

## Types of changes

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)

## Checklist:

- [ ] My code involves a change to the database schema.
- [ ] My code requires a change to the documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laggycomputer laggycomputer laggycomputer approved these changes

@andrew-wang0 andrew-wang0 andrew-wang0 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Courses/Instructors] Cursor based pagination
3 participants
@estao1 @laggycomputer @andrew-wang0