Move wants_thursday_event to custom field #3807
Conversation
📝 WalkthroughWalkthroughThis pull request refactors how the Thursday event is handled in the companies application. The changes remove the translation constant for the Thursday event and introduce a dedicated Boolean field, Changes
Sequence Diagram(s)sequenceDiagram
participant MIG as Migration Script
participant CI as CompanyInterest Record
participant DB as Database
MIG->>CI: Check if "thursday_event" exists in other_offers
CI-->>MIG: Return matching records
MIG->>CI: Remove "thursday_event" from other_offers
MIG->>CI: Set wants_thursday_event = True
MIG->>DB: Persist changes
sequenceDiagram
participant U as User
participant V as CompanyInterestViewSet
participant CSV as CSV Writer
U->>V: Request CSV export
V->>V: Retrieve CompanyInterest data (includes wants_thursday_event)
V->>CSV: Write CSV row with wants_thursday_event field
CSV-->>U: Return CSV output
Suggested labels
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lego/apps/companies/constants.py(0 hunks)lego/apps/companies/migrations/0034_companyinterest_wants_thursday_event.py(1 hunks)lego/apps/companies/models.py(1 hunks)lego/apps/companies/serializers.py(1 hunks)lego/apps/companies/views.py(2 hunks)
💤 Files with no reviewable changes (1)
- lego/apps/companies/constants.py
🧰 Additional context used
🧬 Code Definitions (1)
lego/apps/companies/migrations/0034_companyinterest_wants_thursday_event.py (1)
lego/apps/companies/models.py (1)
CompanyInterest(122-223)
🔇 Additional comments (5)
lego/apps/companies/models.py (1)
161-161: Field addition looks goodThe new Boolean field
wants_thursday_eventhas been added with appropriate default value and blank flag, matching the style of the existingoffice_in_trondheimfield.lego/apps/companies/serializers.py (1)
302-302: Serializer field added correctlyThe new field has been properly added to the serializer fields list, making it accessible through the API. This is an essential step when adding a new model field.
lego/apps/companies/views.py (1)
281-285: Variable creation follows existing patternThe handling of the
wants_thursday_eventfield in the CSV export follows the same pattern as the other boolean fields likeoffice_in_trondheim, which is good for consistency.lego/apps/companies/migrations/0034_companyinterest_wants_thursday_event.py (2)
6-15: Well-structured data migrationThe migration function properly identifies and updates existing records by:
- Finding all company interests that had "thursday_event" in the
other_offersfield- Removing "thursday_event" from that array field
- Setting the new Boolean field to
Truefor those recordsThis is a clean approach that preserves the data while moving it to a dedicated field.
23-30: Migration operations look goodThe migration correctly:
- Adds the new field to the model
- Runs the data migration function to move existing data
- Has proper dependency on the previous migration
This ensures a smooth transition from the old data structure to the new one.
| @@ -292,6 +297,7 @@ def csv(self, *args, **kwargs): | |||
| company_type, | |||
| company_course_themes, | |||
| office_in_trondheim, | |||
| wants_thursday_event, | |||
There was a problem hiding this comment.
Missing CSV header for the new field
While the data row includes the new wants_thursday_event field, there's no corresponding header in the CSV. This will cause misalignment between headers and data columns.
Add the header for the new field by updating the header row in the writer.writerow() call around line 186-210:
writer.writerow(
[
"Navn på bedrift",
"Kontaktperson",
"E-post",
"Telefonnummer",
"Bedriftsinformasjon",
"Semester",
"Arrangementer",
"Annet",
"Samarbeid",
"Bedriftstype",
"Relevante temaer",
"Kontorer i Trondheim for besøk",
+ "Ønsker torsdagsarrangement",
"Klassetrinn",
"Antall deltagere",
"Faglig arrangement kommentar",
"Frokostforedrag kommentar",
"Alternativt arrangement kommentar",
"Start-up kommentar",
"Bedrift-til-bedrift kommentar",
"Lunsjpresentasjon kommentar",
"Bedriftspresentasjon kommentar",
"BedEx kommentarg",
]
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| wants_thursday_event, | |
| writer.writerow( | |
| [ | |
| "Navn på bedrift", | |
| "Kontaktperson", | |
| "E-post", | |
| "Telefonnummer", | |
| "Bedriftsinformasjon", | |
| "Semester", | |
| "Arrangementer", | |
| "Annet", | |
| "Samarbeid", | |
| "Bedriftstype", | |
| "Relevante temaer", | |
| "Kontorer i Trondheim for besøk", | |
| "Ønsker torsdagsarrangement", | |
| "Klassetrinn", | |
| "Antall deltagere", | |
| "Faglig arrangement kommentar", | |
| "Frokostforedrag kommentar", | |
| "Alternativt arrangement kommentar", | |
| "Start-up kommentar", | |
| "Bedrift-til-bedrift kommentar", | |
| "Lunsjpresentasjon kommentar", | |
| "Bedriftspresentasjon kommentar", | |
| "BedEx kommentarg", | |
| ] | |
| ) |
There was a problem hiding this comment.
@ivarnakken Thanks! Happy to help. Let me know if you need any more assistance with this PR.
| @@ -292,6 +297,7 @@ def csv(self, *args, **kwargs): | |||
| company_type, | |||
| company_course_themes, | |||
| office_in_trondheim, | |||
| wants_thursday_event, | |||
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3807 +/- ##
==========================================
- Coverage 89.98% 89.98% -0.01%
==========================================
Files 747 748 +1
Lines 24111 24124 +13
==========================================
+ Hits 21697 21707 +10
- Misses 2414 2417 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
falbru
force-pushed
the
falkbruskeland/aba-1349-move-thursday-button-on-interest-form
branch
2 times, most recently
from
7047a8f to
f8ad35d
Compare
falbru
force-pushed
the
falkbruskeland/aba-1349-move-thursday-button-on-interest-form
branch
from
93afa3c to
1115303
Compare
Description
Frontend: webkom/lego-webapp#5498
Testing
The migration works. Also creation of new company intersts work.
Resolves ... (either GitHub issue or Linear task)