Added Training Codes to be stored, bugfixes #679
Conversation
Added Training Codes to study lines. Bugfixes: - WorkshopBalances are now only recalculated when the Gazdasági Alelnök initiates this change (it should not be triggered automatically). - Decreased the time the current semester is cached to 10 seconds as having cache made debugging extremely difficult. With 10 seconds, the caching has its probable positive effects but it causes interference harder.
📝 WalkthroughWalkthroughThe pull request introduces a comprehensive enhancement to handle training codes for study lines across the application. This involves adding a new Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🔇 Additional comments (1)
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
database/migrations/2025_01_06_183456_add_training_code_to_study_lines.php (1)
15-15: Consider preparing for future non-nullable requirements.
This column is currently marked as nullable, yet the comment hints at making it mandatory in due course. To avoid data inconsistency, you might wish to employ a two-phase migration: firstly introducing the column as nullable, then updating existing records, and finally converting it toNOT NULLin a later migration.database/factories/StudyLineFactory.php (1)
26-26: Promote consistent data generation.
This usage of Faker is beneficial for testing. If reliability is paramount, you might consider generating always non-null codes in certain test scenarios. This will ensure coverage for real-world usage where a code may eventually be mandatory.resources/views/user/study-line-selector.blade.php (2)
4-4: Check UX implications of narrower fields.
The reduction fromxl=6toxl=4could hamper readability if lengthy study line names are entered. Please ensure that this smaller field size remains user-friendly.
20-20: Minor field size adjustment.
Decreasing the minor field toxl=3fromxl=4is a subtle layout change. Kindly confirm it still presents all relevant data clearly.app/Models/StudyLine.php (1)
114-116: Graceful concatenation of the training code.
Appending the training code here is a neat extension of the existinggetNameWithYear()logic. Be mindful of potential whitespace or punctuation nuances in future expansions.app/Http/Controllers/Secretariat/UserController.php (1)
172-172: Prudent validation rule added.
Enforcing'string|max:255'ensures that training codes remain within feasible limits. If the training code is optional, you might consider adding'nullable'for completeness.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
Makefile(1 hunks)app/Http/Controllers/Secretariat/UserController.php(2 hunks)app/Http/Controllers/StudentsCouncil/EconomicController.php(0 hunks)app/Models/Semester.php(1 hunks)app/Models/StudyLine.php(3 hunks)app/Models/User.php(0 hunks)database/factories/StudyLineFactory.php(1 hunks)database/migrations/2025_01_06_183456_add_training_code_to_study_lines.php(1 hunks)resources/lang/en/user.php(1 hunks)resources/lang/hu/user.php(1 hunks)resources/views/auth/application/application.blade.php(1 hunks)resources/views/user/study-line-selector.blade.php(2 hunks)
💤 Files with no reviewable changes (2)
- app/Http/Controllers/StudentsCouncil/EconomicController.php
- app/Models/User.php
🔇 Additional comments (10)
database/migrations/2025_01_06_183456_add_training_code_to_study_lines.php (1)
25-25: Confirm necessity of rollback data.
In the down method, dropping the column is perfectly acceptable. Kindly ensure that any critical information stored intraining_codehas been addressed or migrated elsewhere beforehand, if needed.resources/views/user/study-line-selector.blade.php (1)
14-18: Excellent addition of the training code field.
Adding this new input aligns with your backend changes, ensuring the system now collects and manages thetraining_codeas intended.resources/lang/hu/user.php (1)
79-79: Appreciation for localisation completeness.
Introducing'study_line_training_code' => 'Képzési kód'is an appropriate step towards a fully localised user experience. Thank you for maintaining consistency in translations.resources/lang/en/user.php (1)
79-79: Splendid addition for consistency.
This new key harmonises impeccably with the system's structure, enabling future expansions involving training codes.app/Models/StudyLine.php (2)
18-18: Doc block appears accurate.
This newly introduced@property string $training_codeis well-defined, helping maintain clarity in type-hinting and code completion.
51-51: Proper mass assignment inclusion.
Adding'training_code'to$fillableis essential for seamless creation and updating of study lines.app/Models/Semester.php (1)
280-280: Caching set to 10 seconds.
A cache duration of 10 seconds may significantly increase cache misses, though ideal for debugging. Do ensure this short interval does not hinder production performance.app/Http/Controllers/Secretariat/UserController.php (1)
226-226: Seamless integration of the training code.
Including'training_code'in the study line creation fosters consistent data storage and retrieval.resources/views/auth/application/application.blade.php (1)
167-167: Including year in study line display is splendid
It is quite a marvellous idea to include the year in the study line display. Kindly ensure that if thetraining_codeis also appended within thegetNameWithYear()method, it is being showcased correctly when present.Makefile (1)
15-20: Delightful new target for clearing caches
This splendidclear_cachetarget succinctly clears various Laravel caches, which should greatly facilitate debugging. You may wish to add a brief comment in the Makefile or README clarifying that this command is best utilised in development or while troubleshooting, as frequent cache clearing can degrade performance in a production setting.
|
The Szakmai Alelnök approved the changes in the perspective of a user |
|
Please fix the unit tests: |
horcsinbalint
merged commit 92d587b
into
EotvosCollegium:development
Description
Added Training Codes to study lines and fixed bugs.
Related Issue
Closes #529.
Changes