Conversation
There was a problem hiding this comment.
🔍 Peer Review: Calendar Event Model Implementation
Hey Aviendha👋 — just finished reviewing your PR. Here's my feedback:
✅ What Looks Great
- The model structure has been implemented cleanly.
- Your event type choices are well-organized using class constants.
💡 Suggestions for Improvement (if any)
- Consider adding date validation in the clean method to make sure events aren't scheduled in the past.
- Maybe add a location field. And a field to see if it's a virtual meeting or not.
- For the
event_typefield, consider whether it should really allow null/blank values when it has a default.
🧪 Tested This Feature
I ran the following test steps:
- ✅ Verified model structure and field definitions.
- ✅ Confirmed relationship with Assignment model.
🚀 Final Thoughts
Besides the suggestions I made, lgtm 👍 approved :)
aviendha-andrus
force-pushed
the
calendar_events_model
branch
from
24f8177 to
52bc446
Compare
|
Thank you for your input! I added the clean method, and the location and virtual fields as suggested. |
omgdory
left a comment
omgdory
left a comment
There was a problem hiding this comment.
✅ Approval Overview
Managers added and serve to be really useful. Migrations created. Models make sense, serializers added for data formatting, and URL successfully handled. Views are very succinct and utilize pre-existing Django viewsets, particularly ModelViewSet, meaning that functionality aligns with existing Django methods and documentation. Settings updated to register this as an app. Overall, looks good! All that remains are tests.
🧠 Details
The introduction of the planner application, along with its models and associated views, enriches the functionality of the LessonConnect. The models are well-structured, capturing essential data (titles, dates, times, etc.). This lays the groundwork for a robust event management system within the application.
The addition of CRUD API endpoints demonstrates a clear understanding of RESTful principles and enhances the application's interactivity.
The implementation of features such as virtual event handling and the inclusion of a clean method to validate event data helps ensure data integrity. These features improve the user experience and contribute to the maintainability of the source code.
🎯 Next Steps
Although manual checking via the admin panel and using CURL work, it is important to add automatic tests to ensure that the CI/CD pipeline updates relative to these additional models. As discussed, these tests can be created in a separate PR. As it stands, this looks good to merge, and pre-existing functionality does not seem to have been affected. Great work!
ChrisCRL
left a comment
ChrisCRL
left a comment
There was a problem hiding this comment.
Thanks for adding my suggestions! Everything looks good to me. Approved 👍
|
Hey @aviendha-andrus Great work, just what we needed for our calendar to function properly! I know Abdul has already designed a good-looking calendar on the frontend, and I love the fact that you’ve provided a clear step-by-step guide on how test your work. It’s super helpful for integration. I also appreciate that @AbdulAlharbi has already structured his code to expect where this should connect, so no need to worry about the design being affected. When it’s time to hook everything up, this should align perfectly with our MVP goals and be a strong addition for the presentation. Amazing job, @aviendha-andrus ! If you plan on connecting this to the backend in your upcoming PRs, make sure to reach out to @AbdulAlharbi —he can definitely help smooth out the integration process.
|
|
Hey @aviendha-andrus Just wanted to say how proud I am of the work you’ve done here. This backend implementation is exactly the kind of thoughtful, flexible design we need for LessonConnect to grow into a truly functional platform. You’ve covered both the essentials and future considerations beautifully. I especially appreciate how clearly you outlined the setup steps and testing approach—it’s going to make front-end integration smooth sailing. As PM, it's always a joy to see this level of clarity, coordination, and quality in our core features. This lays a rock-solid foundation for what’s coming next. Excited to see this come together with the frontend! Amazing job! — Abdul |
|
This is a great addition to LessonConnect. Thank you for completing your task and getting this implementation done : D This model is very important towards making progress on the calendar functionality. The instructions you provided on how to test your model is clear and easy to follow. Furthermore, I love how well you documented everything about your implementation so that in the future it'll be easy for us to use in case we want to add more to it. Looking forward to seeing how this continues to evolve, especially once tests are added in the follow-up. Great work!! |
Hi @FrankFurter123! |
Hi @AbdulAlharbi |
Hi @ashley-arellano, |
|
Hello @aviendha-andrus, Since I'm currently working on two calendar-related tasks myself, I was eager to familiarize myself early with your work. Also, I am more than glad to read through this PR. After reviewing the thoughtful discussions you've had with Chris and Dorian, and closely examining your calendar model, I wanted to highlight and express my appreciation! Initial Impression: Great job, and I'll definitely reach out if I need any advice or further clarification while developing my own calendar features. Keep up the awesome work! — Ethan |
I’m so glad to hear your feedback in this! Because we are working on the same topic its important to keep each other updated so I’m grateful you saw my work and like what you see. I’ll keep an eye out for your work too and make sure I stay updated with it. |
7 participants
Calendar Events Model
Added models and API endpoints for calendar events
Overview
What does this PR do?
This PR introduces a fully functional Calendar model feature to LessonConnect, allowing users to create events. The calendar supports different types of events including lessons, meetingss, reminders, and practice/study sessions.
Key Features & Changes
What has been added or changed?
Example:
title,date,start_time,end_time, anddescription.Why This Is Needed
What problem does this solve?
A flexible calendar is a neccisary component of LessonConnet, so that parents, students, and teachers can stay in the loop and on top of events and assignments.
Related Issue
🔗 This PR closes #100
Implementation Details
How was this feature developed?
Design Documentation (If Applicable)
Diagrams & Visuals
How to Test This PR
Step-by-step testing guide
1️⃣ Acquire an access token: To do this, migrate and run the server like usual (python manage runserver). Then go to http://127.0.0.1:8000/api/token/ via the browser. Sign in with your superuser account info and copy your access token.
2️⃣ Keep the server running in one terminal.
3️⃣ Note this must be done in another separate (not in the virtual environment) terminal. Paste access token into curl commands that test lesson-related API.
4️⃣ Fill in any desired API endpoint Fields, some being required while others not (and some being avoided more will be listed out in follow-up PR).
5️⃣ Outputs will be in the second (not in the virtual environment) terminal.
Files Added/Modified
📂 List of important files changed in this PR
models.pyserializers.pyviews.pyurls.pybackend/urls.pyadmin.pymanagers.pyAI Code Usage
🤖 Was AI-generated code used in this PR?
Checklist for Reviewers
Things to check before approving this PR:
Next Steps & Future Enhancements
What improvements can be made after merging?
Final Notes
Thank you for reviewing! Please leave feedback if you have any suggestions or concerns. Let’s make this feature even better!