Skip to content

feat: implement CRUD API w/ testing#2

Merged
jordan-smith721 merged 3 commits intomongodb:mainfrom
jordan-smith721:api-crud
Oct 16, 2025
Merged

feat: implement CRUD API w/ testing#2
jordan-smith721 merged 3 commits intomongodb:mainfrom
jordan-smith721:api-crud

Conversation

@jordan-smith721
Copy link
Collaborator

Implements API endpoints for CRUD operations w/ unit testing

Copy link
Collaborator

@cbullinger cbullinger left a comment

Choose a reason for hiding this comment

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

A couple questions and suggestions

@jordan-smith721 jordan-smith721 marked this pull request as ready for review October 14, 2025 19:37
Copy link

@baileympearson baileympearson left a comment

Choose a reason for hiding this comment

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

Just a few small comments

Comment on lines 96 to 107
// Parse and validate pagination parms for invalid inputs
const limitNum = Math.min(
Math.max(
parseInt(limit) || 20, // Default to 20 if invalid
1 // Min 1 result
),
100 // Cap at 100 results for performance
);
const skipNum = Math.max(
parseInt(skip) || 0, // Default to 0 if invalid
0 // skip must be positive number
);

Choose a reason for hiding this comment

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

minor comment - indentation is strange here. should we add a formatter to the project?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah good call. Formatted all files w/ Prettier


// Prepare the document for insertion
// Here you can add metadata or other fields that might be necessary
const movieDocument: Partial<Movie> = {

Choose a reason for hiding this comment

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

Is there a reason to clone movieData?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea was just in case someone wanted to add metadata or something if they edit this app, but thinking about that now that seems like overkill for this so I removed it

Comment on lines 194 to 196
// Retrieve the created document to return complete data
const createdMovie = await moviesCollection.findOne({ _id: result.insertedId });

Choose a reason for hiding this comment

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

Why is this necessary? Can't you just return movieData?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did it this way so that it retrieves the actual full document that got inserted, rather than returning what the user submits. At the end of the day it'll likely be the same info, but this seemed like a better way to show that the document was actually inserted.
Happy to edit it if you feel strongly, I'm not tied to doing either way

@jordan-smith721 jordan-smith721 merged commit 0dac316 into mongodb:main Oct 16, 2025
1 check passed
cbullinger added a commit that referenced this pull request Jan 31, 2026
- #1 Movie Cards: Make entire card clickable, enforce consistent heights, tone down checkbox
- #2 Top Toolbar: Remove batch buttons, add contextual bottom selection bar
- #3 Filters Bar: Replace mint/green with neutral gray borders and backgrounds
- #4 Navbar: Remove full-width green border and animated underline effect
- #5 Aggregations: Use light gray for row hover, tone down comment pills and show more button
- Additional: Remove bright green border from aggregations section headers

All changes improve visual hierarchy and reduce competing visual elements per reviewer feedback on PR #75.
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.

3 participants