feat: implement movie details page and optimize components#9
Conversation
cbullinger
left a comment
There was a problem hiding this comment.
Overall looking good! A few questions for you (fyi - i only skimmed most of the actual frontend elements, but they all displayed nicely for me, so 🤷 )
|
|
||
| ## Features | ||
|
|
||
| ### ✅ Implemented CRUD Operations |
There was a problem hiding this comment.
| ### ✅ Implemented CRUD Operations | |
| ### Implemented CRUD Operations |
There was a problem hiding this comment.
Removed ReadME
| - **deleteMany()** - Delete multiple movies based on filter criteria | ||
| - **findOneAndDelete()** - Find and delete a movie in one atomic operation | ||
|
|
||
| ### 🔮 Planned Features (Post-MVP) |
There was a problem hiding this comment.
| ### 🔮 Planned Features (Post-MVP) | |
| ### Planned Features (Post-MVP) |
| - ✅ MongoDB connection | ||
| - ✅ Sample_mflix database exists | ||
| - ✅ Required collections (movies, theaters, comments) exist | ||
| - ✅ Necessary indexes are created | ||
| - ✅ Sample data is present |
There was a problem hiding this comment.
| - ✅ MongoDB connection | |
| - ✅ Sample_mflix database exists | |
| - ✅ Required collections (movies, theaters, comments) exist | |
| - ✅ Necessary indexes are created | |
| - ✅ Sample data is present | |
| - MongoDB connection | |
| - Sample_mflix database exists | |
| - Required collections (movies, theaters, comments) exist | |
| - Necessary indexes are created | |
| - Sample data is present |
| const validateForm = () => { | ||
| const newErrors: Record<string, string> = {}; | ||
|
|
||
| // For batch updates, we only validate if fields have values |
There was a problem hiding this comment.
do we want to rephrase this slightly for clarity that we're only validating fields with values if those fields have valid values to validate against? (if that makes sense) e.g. there's no validation for the rating since there's no global standard
There was a problem hiding this comment.
Updated to:
For batch updates, we only validate if a field has a value and if that value must meet certain criteria.
| updateData.poster = formData.poster.trim(); | ||
| } | ||
|
|
||
| // Check if there's actually something to update |
There was a problem hiding this comment.
| // Check if there's actually something to update | |
| // Check if user entered anything for the batch update |
There was a problem hiding this comment.
and idk if it matters but this validation only applies to the batch update. you can successfully click Update on a single movie without having made any changes (but you can't do the same by batch updating that same single movie)
There was a problem hiding this comment.
Good catch, added to the edit form as well
| /> | ||
| </div> | ||
|
|
||
| {/* Lists (comma-separated) */} |
There was a problem hiding this comment.
should the placeholders for all these be preceded by e.g., like the rating field?
| 'use client'; | ||
|
|
||
| /** | ||
| * Add Movie Form Component |
There was a problem hiding this comment.
do we care about an upper limit on this? currently, you can add infinite movies
There was a problem hiding this comment.
I don't think we need a limit for now since this is just a "sample"
| /> | ||
| </div> | ||
|
|
||
| {/* Lists (comma-separated) */} |
There was a problem hiding this comment.
same as earlier comment re: adding e.g., to placeholders here
| */ | ||
| export async function updateMoviesBatch(movieIds: string[], updateData: Partial<Movie>): Promise<{ success: boolean; error?: string; matchedCount?: number; modifiedCount?: number }> { | ||
| try { | ||
| // Create filter to match the movie IDs |
There was a problem hiding this comment.
| // Create filter to match the movie IDs | |
| // Create filter to match the movie IDs | |
| // Note: The server will handle ObjectId conversion |
applicable here as well?
| setSuccessMessage('Movie created successfully!'); | ||
| setShowAddForm(false); | ||
|
|
||
| // If we have a movieId, redirect to the new movie's page after a brief delay |
There was a problem hiding this comment.
is there a scenario where we don't have a movieId but the creation was successful?
There was a problem hiding this comment.
Nope! Removed
Adds movie details page which finds a movie by ID. Also adds create, edit, and delete functionality, as well as bulk delete