-
Notifications
You must be signed in to change notification settings - Fork 15
Refine userpolicy and allow partial fillings in application #746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis update introduces detailed field-level validation and permission checks for user and application data management. It refines the UserPolicy with new granular authorisation methods, enhances validation logic and error reporting, adjusts database schema for nullable fields, and updates user interface components to reflect required fields and permissions. Related tests and translations are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant UserController
participant UserPolicy
participant ApplicationController
participant Models
User->>UI: Attempts to edit user/application data
UI->>UserController: Submits form data
UserController->>UserPolicy: Checks 'edit' or related permissions
alt Permission granted
UserController->>Models: Validates and updates data
Models-->>UserController: Returns success/failure
UserController-->>UI: Displays result
else Permission denied
UserController-->>UI: Displays error or disables fields
end
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🧹 Nitpick comments (6)
resources/views/components/input/checkbox.blade.php (1)
15-18
: Functionality approved, though consider CSS class for styling consistency.The conditional asterisk rendering works correctly and follows the expected pattern. However, the inline
style="color:red;"
approach might be worth standardising with a CSS class for better maintainability across the application.resources/views/auth/application/files.blade.php (1)
20-23
: Asterisk indicator properly implemented, but consider internationalization.The required field indicator works correctly. However, the change from a translation key to hardcoded Hungarian text (
"Fájl kiválasztása"
) may pose internationalization concerns if the application supports multiple languages.app/Models/EducationalInformation.php (1)
130-130
: Consider the implications of exposing this method publicly.Whilst the visibility change enables external permission checks, it exposes internal business logic that was previously encapsulated. This could lead to inconsistent usage patterns across the codebase.
Consider whether a more structured approach would be preferable, such as:
- Creating a dedicated service class for permission-related checks
- Adding a facade method that provides a cleaner interface
- Using Laravel's authorization system more extensively
app/Http/Controllers/Auth/ApplicationController.php (1)
153-153
: Consider using Carbon for date handling consistency.Whilst
date('Y')
is functional, using Carbon would provide better consistency with the rest of the Laravel application and make the code more testable.- 'year_of_acceptance' => date('Y') + 'year_of_acceptance' => now()->yearresources/lang/hu/user.php (1)
23-23
: Consider aligning contact options across languages.The Hungarian version provides more comprehensive contact information (system administrators, student council, and secretariat) compared to the English version which only mentions system administrators. This inconsistency might confuse users.
Consider updating the English version to match the Hungarian comprehensiveness:
-'data_cannot_be_edited_application' => 'To edit your information, contact the [system administrators](mailto::SYSADMIN_EMAIL).', +'data_cannot_be_edited_application' => 'Due to the admission process, you currently cannot edit your information. To make changes, please contact the [system administrators](mailto::SYSADMIN_EMAIL), the [Student Council](mailto::STUDENT_COUNCIL_EMAIL), or the [Secretariat](mailto::SECRETARIAT_EMAIL).',resources/views/user/alfonso.blade.php (1)
39-42
: Consider consistency in disabled state styling.The form controls properly implement permission-based disabling and required field indicators. However, ensure that disabled form elements have appropriate visual styling to clearly indicate their non-editable state to users.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
app/Http/Controllers/Auth/ApplicationController.php
(1 hunks)app/Http/Controllers/Secretariat/UserController.php
(14 hunks)app/Models/Application.php
(1 hunks)app/Models/EducationalInformation.php
(1 hunks)app/Policies/UserPolicy.php
(2 hunks)app/Utils/ApplicationHandler.php
(2 hunks)app/Utils/HasRoles.php
(1 hunks)database/migrations/2025_07_11_184933_make_phone_number_nullable.php
(1 hunks)database/migrations/2025_07_11_202220_make_fields_in_study_lines_nullable.php
(1 hunks)resources/lang/en/user.php
(2 hunks)resources/lang/hu/user.php
(2 hunks)resources/views/auth/application/app.blade.php
(1 hunks)resources/views/auth/application/files.blade.php
(1 hunks)resources/views/auth/application/questions.blade.php
(6 hunks)resources/views/components/input/checkbox.blade.php
(1 hunks)resources/views/components/input/datepicker.blade.php
(2 hunks)resources/views/components/input/file.blade.php
(1 hunks)resources/views/components/input/select.blade.php
(1 hunks)resources/views/components/input/text.blade.php
(1 hunks)resources/views/components/input/textarea.blade.php
(1 hunks)resources/views/components/input/timepicker.blade.php
(1 hunks)resources/views/user/alfonso-language-exams.blade.php
(1 hunks)resources/views/user/alfonso.blade.php
(1 hunks)resources/views/user/educational-information.blade.php
(3 hunks)resources/views/user/personal-information.blade.php
(1 hunks)resources/views/user/study-line-selector.blade.php
(1 hunks)resources/views/user/update_tenant_status.blade.php
(1 hunks)resources/views/utils/user/profile-picture.blade.php
(3 hunks)tests/Feature/ApplicationTest.php
(6 hunks)tests/Feature/UserTest.php
(0 hunks)tests/Unit/AlfonsoTest.php
(1 hunks)
💤 Files with no reviewable changes (1)
- tests/Feature/UserTest.php
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: horcsinbalint
PR: EotvosCollegium/mars#743
File: app/Http/Controllers/Auth/AdmissionController.php:237-237
Timestamp: 2025-07-08T22:59:53.901Z
Learning: When horcsinbalint defers issues for "human reviewers to check later," he's making a conscious decision to prioritize the current PR over fixing secondary issues, and this should be acknowledged rather than being insistent about immediate fixes.
app/Utils/ApplicationHandler.php (1)
Learnt from: viktorcsimma
PR: EotvosCollegium/mars#542
File: app/Http/Controllers/Secretariat/SemesterEvaluationController.php:161-161
Timestamp: 2024-06-13T19:25:19.821Z
Learning: 'anonymous_questions' is a section name in the 'secretariat.evaluation-form.app' view and does not require handling in the `store` method of `SemesterEvaluationController.php`.
resources/views/auth/application/questions.blade.php (1)
Learnt from: viktorcsimma
PR: EotvosCollegium/mars#542
File: app/Http/Controllers/Secretariat/SemesterEvaluationController.php:161-161
Timestamp: 2024-06-13T19:25:19.821Z
Learning: 'anonymous_questions' is a section name in the 'secretariat.evaluation-form.app' view and does not require handling in the `store` method of `SemesterEvaluationController.php`.
🧬 Code Graph Analysis (4)
database/migrations/2025_07_11_184933_make_phone_number_nullable.php (1)
database/migrations/2025_07_11_202220_make_fields_in_study_lines_nullable.php (2)
up
(11-16)down
(21-26)
tests/Unit/AlfonsoTest.php (1)
app/Models/Semester.php (1)
Semester
(51-373)
app/Utils/HasRoles.php (2)
app/Models/User.php (1)
roles
(218-222)app/Models/Role.php (1)
Role
(34-335)
database/migrations/2025_07_11_202220_make_fields_in_study_lines_nullable.php (1)
database/migrations/2025_07_11_184933_make_phone_number_nullable.php (2)
up
(11-16)down
(21-26)
🪛 GitHub Actions: CI
tests/Feature/ApplicationTest.php
[error] 249-262: PHPUnit test failure in test_submit: Expected response status code 302 but received 500 due to exception in UserController.
app/Http/Controllers/Secretariat/UserController.php
[error] 230-230: Error: Call to a member function has() on array causing HTTP 500 response instead of expected 302 in test_submit.
🪛 PHPMD (2.15.0)
app/Http/Controllers/Secretariat/UserController.php
266-266: Avoid unused local variables such as '$data'. (Unused Code Rules)
(UnusedLocalVariable)
🔇 Additional comments (44)
tests/Unit/AlfonsoTest.php (1)
45-45
: Excellent visibility change for improved code reuse.Making this method public allows other test classes to utilise this utility function, promoting consistency across the test suite. The method logic remains sound and unchanged.
app/Utils/HasRoles.php (1)
202-205
: Well-implemented utility method for role-based authorization.The method provides a clear and focused check for users with roles beyond TENANT. The implementation is straightforward and the naming is descriptive. This will be particularly useful for authorization policies.
resources/views/components/input/textarea.blade.php (1)
10-12
: Proper implementation of required field visual indicator.The conditional red asterisk follows standard UX patterns and maintains consistency with other input components. This enhancement will improve user experience by clearly indicating mandatory fields.
resources/views/auth/application/app.blade.php (1)
89-89
: Improved button text clarity.The change from "Ellenőrzés" to "Áttekintés" better describes the button's function, as "overview" is more accurate than "check" for a review and finalisation step. This enhances user understanding of the interface.
resources/views/user/update_tenant_status.blade.php (1)
20-23
: Excellent frontend validation alignment with backend constraints.The date constraints properly prevent users from selecting invalid dates, whilst the null-safe operator ensures robust value binding. This frontend-backend consistency enhances both user experience and data integrity.
resources/views/components/input/datepicker.blade.php (1)
33-42
: Excellent enhancement for date validation constraints.The addition of conditional
maxDate
andminDate
configuration is a splendid improvement that enhances form validation and user experience. The implementation properly checks for attribute presence and converts values to JavaScript Date objects.This change provides valuable date range validation that will prevent users from selecting invalid dates, which is particularly important for fields like tenant dates as mentioned in the AI summary.
database/migrations/2025_07_11_184933_make_phone_number_nullable.php (2)
11-16
: Migration structure is sound and aligns with PR objectives.The nullable change to
phone_number
properly supports the partial application filling requirement mentioned in the PR objectives, allowing users to save incomplete forms whilst deferring validation until full submission.
21-26
: Down method correctly implements rollback logic.The reversion to non-nullable state is properly implemented, ensuring the migration can be safely rolled back if needed.
database/migrations/2025_07_11_202220_make_fields_in_study_lines_nullable.php (2)
11-16
: Migration follows established pattern and supports partial filling.The nullable change to the
name
field instudy_lines
is consistent with the similar migration forphone_number
and properly supports the partial application filling feature described in the PR objectives.
21-26
: Rollback logic correctly implemented.The down method properly reverts the nullable change, maintaining migration integrity.
resources/views/auth/application/files.blade.php (1)
24-26
: Consistent asterisk implementation for name field.The asterisk attribute addition maintains consistency with the file input above and follows the established pattern across input components.
resources/views/user/alfonso-language-exams.blade.php (2)
45-45
: Excellent improvement to use proper authorization.The change from checking
$user->educationalInformation
to using@can('edit', $user)
is a significant improvement that properly implements the refined UserPolicy mentioned in the PR objectives, ensuring consistent permission checks across the application.
50-65
: Consistent asterisk implementation across form inputs.The asterisk attributes have been properly added to all required fields (file, language, level, type, date), maintaining consistency with the UI improvements implemented across other input components in this PR.
resources/lang/en/user.php (2)
72-72
: LGTM! Clear and helpful user messaging.The language entries provide clear guidance to users about editing restrictions and appropriate contact channels.
23-23
: Email placeholder replacement mechanism is secureI’ve confirmed that the
::SYSADMIN_EMAIL
,::SECRETARIAT_EMAIL
and::STUDENT_COUNCIL_EMAIL
tokens are never sourced from user input but from trusted config entries (config('mail.sys_admin_mail')
, etc.). They’re applied in two ways:
- For configurable texts, the
ConfigurableText
model’sget
accessor runs a simplestr_replace
on the raw markdown, swapping each placeholder for its config value.- For user language strings, the Blade
@markdown
directive receives the translation (which still contains::SYSADMIN_EMAIL
) and a second argument array of the same config values. This array is passed into the markdown-to-HTML renderer, which produces safe<a href="mailto:…">
links via the Markdown parser. There is no opportunity for untrusted input or HTML injection.No sanitisation gaps were found, and no further changes are required.
app/Http/Controllers/Auth/ApplicationController.php (1)
147-156
: LGTM! Proper data integrity enforcement.The extension to ensure related user data entities exist is well-implemented and follows the established pattern. The automatic creation of
personalInformation
andeducationalInformation
supports the partial filling feature effectively.resources/views/user/study-line-selector.blade.php (2)
7-11
: Excellent implementation of permission-based UI controls.The conditional requirements and permission checks are properly implemented. The asterisk indicators and disabled states provide clear visual feedback to users about field requirements and editing restrictions.
57-59
: Proper authorization for destructive actions.The
@can('edit', $user)
directive correctly restricts the delete button to authorised users only, preventing unauthorised data modification.resources/lang/hu/user.php (1)
76-76
: LGTM! Improved translation specificity.The refinement from "Szak" to "Szak neve" provides better clarity for users about what information is expected in this field.
resources/views/user/alfonso.blade.php (2)
18-34
: Excellent implementation of granular permission checks.The conditional display of informational messages based on user permissions is well-structured. The separation between general edit permissions and static educational information editing provides appropriate access control granularity.
50-52
: Proper authorization enforcement on form submission.The conditional rendering of the save button based on permissions is correct and prevents unauthorised form submissions. This aligns well with the backend authorization checks.
app/Utils/ApplicationHandler.php (3)
22-32
: Well-implemented nullable validation rules for partial completion.The changes to make fields nullable support the PR objective of allowing partial application completion. The validation rules properly handle optional data whilst maintaining data integrity.
43-52
: Robust null handling for status field.The status handling logic correctly manages null values, which supports partial application completion. The explicit null assignment when status is not set is appropriate for the new nullable approach.
40-40
: Validation rule for ‘accommodation’ is correctThe Blade checkbox in
resources/views/auth/application/questions.blade.php
submits a boolean-like value (“on” when checked), which thesometimes|accepted
rule rightly validates. The handler’s line$data['accommodation'] = isset($data['accommodation']) && $data['accommodation'];and the model’s boolean cast ensure the database field remains in sync. No further changes required.
resources/views/utils/user/profile-picture.blade.php (3)
9-19
: Proper authorization implementation for profile picture deletion.The permission check using
@can('edit', $user)
correctly restricts profile picture deletion to authorised users. The consistent implementation across both desktop and mobile views is commendable.
24-35
: Consistent authorization for profile picture upload.The upload form is properly wrapped with permission checks, ensuring only authorised users can modify profile pictures. The form structure and validation remain intact whilst adding security controls.
30-31
: Improved user guidance with descriptive labels.The updated file input text "Profilkép választása - nem kötelező" provides clearer guidance to users about the optional nature of profile pictures. This enhances the user experience.
Also applies to: 53-53
resources/views/auth/application/questions.blade.php (4)
12-12
: Excellent visual indicators for required fields.The addition of red asterisks to indicate required fields significantly improves user experience by clearly communicating mandatory information. This visual enhancement aligns well with the form validation approach.
Also applies to: 47-47, 72-72, 91-91
122-142
: Improved question input structure with explicit labels.The conversion from x-input components to explicit labels and textareas provides better semantic structure and accessibility. The consistent styling with
font-size: 15px
ensures visual consistency.
145-148
: Enhanced textarea with proper constraints.The addition of
maxlength="5000"
and improved helper text formatting provides better user guidance and data validation. This enhancement supports proper form completion.
51-51
: Ensure server-side validation for the “status” radio groupI wasn’t able to find any server-side rule validating the
status
field in your FormRequest classes or controllervalidate()
calls. Please confirm that your final submission request includes something like:'status' => 'required|in:resident,passive,alumni,…'Key locations to check:
- app/Http/Controllers/Auth/AdmissionController.php (currently only validates a
status_filter
)- app/Http/Controllers/Secretariat/SemesterEvaluationController.php (validates
section
, notstatus
)- Any FormRequest under app/Http/Requests (directory does not appear to exist)
resources/views/user/educational-information.blade.php (5)
3-19
: Exemplary implementation of layered permission messaging.The conditional display of informational messages based on different permission levels (
edit
vseditStaticEducationalInformation
) provides excellent user feedback. The structured approach to permission-based UI is commendable.
28-32
: Consistent permission-based field control implementation.The systematic application of permission checks, required indicators, and validation attributes across input fields demonstrates excellent attention to detail. The conditional requirements based on user roles (collegist, alumni, application presence) are well-implemented.
Also applies to: 36-39, 40-46
47-55
: Robust validation for Neptun code field.The Neptun code field implementation with proper regex pattern validation (
^$|^[A-Za-z0-9]{6}$
) and length constraints demonstrates good data validation practices. The pattern allows for empty values or exactly 6 alphanumeric characters.
76-78
: Proper authorization for checkbox controls.The permission-based disabling of faculty and workshop checkboxes ensures consistent access control across all form elements. The implementation maintains data integrity whilst providing appropriate user feedback.
Also applies to: 94-96
109-111
: Consistent conditional rendering of action elements.The conditional rendering of the "add study line" button and save button based on user permissions ensures UI consistency and prevents unauthorised actions. This approach aligns well with the backend authorization checks.
Also applies to: 130-133
resources/views/user/personal-information.blade.php (3)
8-24
: Well-implemented permission-based messagingThe conditional display of informational messages based on user permissions is properly structured and provides clear guidance to users about their editing capabilities.
25-159
: Properly implemented field-level permission controlsThe dynamic disabling of form inputs and conditional required attributes based on user permissions is well-executed. The distinction between
edit
andeditStaticPersonalInformation
permissions provides appropriate granularity.
162-164
: Appropriate save button access controlThe conditional rendering of the save button based on edit permission prevents unauthorised form submissions at the UI level.
app/Models/Application.php (1)
336-421
: Excellent refactoring of missing data validationThe transformation from broad presence checks to detailed field-level validation significantly improves the user experience by providing specific feedback about missing data. The conditional ALFONSÓ validation based on admission year and exemption status demonstrates thoughtful implementation.
app/Policies/UserPolicy.php (1)
43-93
: Well-structured layered authorization methodsThe three new authorization methods provide appropriate granularity for controlling user data editing permissions. The hierarchical structure, where more specific methods build upon the base
edit
method, follows good design principles.tests/Feature/ApplicationTest.php (1)
185-367
: Test updates align with validation changesThe transformation of test assertions from broad categories to individual field checks properly validates the refactored
missingData()
method. However, the pipeline failure indicates an issue with the educational information update endpoint.The pipeline failure at lines 249-262 suggests the educational information update is returning HTTP 500 instead of 302. This is likely related to the issue in
UserController::updateEducationalInformation()
at line 230.app/Http/Controllers/Secretariat/UserController.php (2)
60-60
: Appropriate elevation of authorization requirementsThe change from 'view' to 'edit' permissions across multiple methods correctly implements the stricter access control requirements for modifying user data.
Also applies to: 85-85, 105-105, 159-159, 263-263, 286-286, 318-318, 341-341
108-130
: Well-designed dynamic validation rulesThe conditional building of validation rules based on user permissions is excellently implemented. This ensures that only editable fields are validated whilst maintaining data integrity for restricted fields.
Also applies to: 161-193
@@ -6,6 +6,7 @@ | |||
use App\Models\RoleObject; | |||
use App\Models\User; | |||
use App\Models\Workshop; | |||
use App\Http\Controllers\Auth\ApplicationController; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Architectural concern: Controller dependency in policy
Importing and instantiating a controller within a policy violates the separation of concerns principle. The ApplicationController::isActive()
check should be refactored to use a service or helper method instead.
Consider extracting the application period logic to a dedicated service:
-use App\Http\Controllers\Auth\ApplicationController;
+use App\Services\ApplicationService;
And in the edit method:
- app(ApplicationController::class)->isActive() &&
+ app(ApplicationService::class)->isApplicationPeriodActive() &&
Also applies to: 52-55
🤖 Prompt for AI Agents
In app/Policies/UserPolicy.php at line 9 and also lines 52-55, remove the import
and usage of ApplicationController within the policy to avoid controller
dependency. Refactor by extracting the application period logic currently in
ApplicationController::isActive() into a dedicated service or helper class.
Then, update the policy to call this new service method instead of the
controller method, maintaining separation of concerns and proper architecture.
921cf5f
to
f492c3e
Compare
4db35c2
to
5271cb1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements partial filling support for application forms and refines user permissions for data editing. Users can now save partially completed forms during the application process, with validation only occurring upon final submission. The PR also restricts data editing once applications are submitted and updates the UserPolicy to properly control who can edit user data.
- Enables partial form filling during application process with validation only on submission
- Restricts user data editing based on application status and user roles
- Updates UserPolicy to allow editing only for specific roles (secretariat, student council leaders, sysadmins) plus the user themselves under certain conditions
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
tests/Unit/AlfonsoTest.php | Changed autumnSemester method visibility from private to public |
tests/Feature/UserTest.php | Removed automatic role assignment and application creation in test setup |
tests/Feature/ApplicationTest.php | Updated test assertions to check for specific missing data fields instead of general categories |
resources/views/utils/user/profile-picture.blade.php | Added authorization checks for profile picture editing |
resources/views/user/update_tenant_status.blade.php | Added date range validation for tenant until field |
resources/views/user/study-line-selector.blade.php | Added conditional requirements and edit permissions for study line fields |
resources/views/user/personal-information.blade.php | Added edit permission checks and conditional field requirements |
resources/views/user/educational-information.blade.php | Added edit permission checks and conditional field requirements |
resources/views/user/alfonso.blade.php | Added edit permission checks and conditional requirements for Alfonso language selection |
resources/views/user/alfonso-language-exams.blade.php | Added edit permission checks for language exam uploads |
resources/views/components/input/*.blade.php | Added asterisk support for required field indicators |
resources/views/auth/application/questions.blade.php | Updated form validation and field requirements for application questions |
resources/views/auth/application/files.blade.php | Added asterisk indicators for required file upload fields |
resources/views/auth/application/app.blade.php | Changed button text from "Ellenőrzés" to "Áttekintés" |
resources/lang/hu/user.php | Added new translation keys for data editing restrictions |
resources/lang/en/user.php | Added new translation keys for data editing restrictions |
database/migrations/*.php | Made phone number and study line name fields nullable |
app/Utils/HasRoles.php | Added hasRoleOtherThanTenant method |
app/Utils/ApplicationHandler.php | Updated validation rules to allow nullable fields during partial filling |
app/Policies/UserPolicy.php | Added new authorization methods for different levels of user data editing |
app/Models/EducationalInformation.php | Changed isMasterAdmittee method visibility from private to public |
app/Models/Application.php | Refactored missingData method to provide more granular validation |
app/Http/Controllers/Secretariat/UserController.php | Updated authorization checks and validation logic throughout |
app/Http/Controllers/Auth/ApplicationController.php | Added automatic creation of personal and educational information records |
$old_profile->update(['path' => $path]); | ||
Storage::delete($old_profile->path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old profile picture file is deleted after updating the database record with the new path. This creates a race condition where if the update fails, the old file is already deleted. The Storage::delete should be called before the update, or the operations should be wrapped in a transaction.
$old_profile->update(['path' => $path]); | |
Storage::delete($old_profile->path); | |
DB::transaction(function () use ($old_profile, $path) { | |
$old_profile->update(['path' => $path]); | |
Storage::delete($old_profile->path); | |
}); |
Copilot uses AI. Check for mistakes.
"level" => "bachelor", | ||
"start" => Semester::current()->id, | ||
"type" => "bachelor", | ||
"start" => AlfonsoTest::autumnSemester()->id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a test helper method from a different test class (AlfonsoTest) creates tight coupling between test classes. Consider moving the autumnSemester method to a shared test utility class or creating the semester data directly in this test.
Copilot uses AI. Check for mistakes.
'end' => $studyLine["end"] ?? null, | ||
]); | ||
} | ||
} | ||
|
||
}); | ||
|
||
return redirect()->back()->with('message', __('general.successful_modification')); | ||
if ($error_with_neptun_code) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The neptun code validation logic is complex and spans multiple lines with nested conditions. This could be extracted into a separate method to improve readability and testability.
Copilot uses AI. Check for mistakes.
required | ||
:disabled="user()->cannot('editStaticPersonalInformation', $user)" | ||
asterisk | ||
maxlength="225" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The maxlength value "225" appears to be inconsistent with the validation rule "max:225" in the controller, but more importantly, the standard maximum length for email addresses is typically 254 characters according to RFC specifications.
maxlength="225" | |
maxlength="254" |
Copilot uses AI. Check for mistakes.
$validator['high_school'] = [$requiredForCollegist, 'string', 'max:255']; | ||
$validator['year_of_graduation'] = [$requiredForCollegist, 'integer', 'between:1895,' . date('Y')]; | ||
$validator['year_of_acceptance'] = [$requiredForCollegist, 'integer', 'between:1895,' . date('Y')]; | ||
$validator['neptun'] = [$requiredForCollegist, 'string', 'size:6']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The neptun validation requires exactly 6 characters when the user is a collegist, but the HTML pattern in the view allows for empty strings (^$|^[A-Za-z0-9]{6}$). This creates an inconsistency between frontend and backend validation.
$validator['neptun'] = [$requiredForCollegist, 'string', 'size:6']; | |
$validator['neptun'] = [$requiredForCollegist, 'string', $requiredForCollegist === 'required' ? 'size:6' : 'nullable']; |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
resources/views/user/educational-information.blade.php (1)
47-68
: Consider simplifying the complex conditional logic.The Neptun code and educational email fields contain multiple conditional expressions that could benefit from refactoring for improved readability.
Consider extracting the conditional logic into computed variables:
+@php + $isApplicationContext = isset($application); + $emailHelper = $isApplicationContext + ? 'lehetőleg @student.elte.hu-s (nem kötelező, a felvételit követően pótolható)' + : 'lehetőleg @student.elte.hu-s'; +@endphp <x-input.text s=6 id='educational-email' text='user.educational-email' name="email" :value="$user->educationalInformation?->email" - :helper=" - isset($application) - ? - 'lehetőleg @student.elte.hu-s (nem kötelező, a felvételit követően pótolható)' - : - 'lehetőleg @student.elte.hu-s'" + :helper="$emailHelper" :required="$user->isCollegist()" :disabled="user()->cannot('edit', $user)" - :asterisk="!isset($application)" + :asterisk="!$isApplicationContext" maxlength="255" />app/Http/Controllers/Secretariat/UserController.php (1)
205-211
: Thoughtful error handling for unique constraint violations.The approach to handle Neptun code uniqueness violations by saving other data whilst flagging the error is user-friendly and prevents data loss. However, consider using more descriptive variable naming.
Consider using a more descriptive variable name:
-$error_with_neptun_code = false; +$neptunCodeConflict = false;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
app/Http/Controllers/Auth/ApplicationController.php
(1 hunks)app/Http/Controllers/Secretariat/UserController.php
(14 hunks)app/Models/Application.php
(1 hunks)app/Policies/UserPolicy.php
(2 hunks)app/Utils/ApplicationHandler.php
(2 hunks)app/Utils/HasRoles.php
(1 hunks)database/migrations/2025_07_11_184933_make_phone_number_nullable.php
(1 hunks)database/migrations/2025_07_11_202220_make_fields_in_study_lines_nullable.php
(1 hunks)resources/views/auth/application/app.blade.php
(1 hunks)resources/views/auth/application/files.blade.php
(1 hunks)resources/views/auth/application/questions.blade.php
(6 hunks)resources/views/components/input/checkbox.blade.php
(1 hunks)resources/views/components/input/datepicker.blade.php
(2 hunks)resources/views/components/input/file.blade.php
(1 hunks)resources/views/components/input/select.blade.php
(1 hunks)resources/views/components/input/text.blade.php
(1 hunks)resources/views/components/input/textarea.blade.php
(1 hunks)resources/views/components/input/timepicker.blade.php
(1 hunks)resources/views/user/alfonso-language-exams.blade.php
(1 hunks)resources/views/user/alfonso.blade.php
(1 hunks)resources/views/user/educational-information.blade.php
(3 hunks)resources/views/user/personal-information.blade.php
(1 hunks)resources/views/user/study-line-selector.blade.php
(1 hunks)resources/views/user/update_tenant_status.blade.php
(1 hunks)resources/views/utils/user/profile-picture.blade.php
(3 hunks)tests/Feature/ApplicationTest.php
(6 hunks)tests/Feature/UserTest.php
(0 hunks)tests/Unit/AlfonsoTest.php
(1 hunks)
💤 Files with no reviewable changes (1)
- tests/Feature/UserTest.php
✅ Files skipped from review due to trivial changes (1)
- resources/views/components/input/timepicker.blade.php
🚧 Files skipped from review as they are similar to previous changes (24)
- resources/views/auth/application/app.blade.php
- resources/views/components/input/select.blade.php
- resources/views/components/input/file.blade.php
- resources/views/components/input/datepicker.blade.php
- database/migrations/2025_07_11_202220_make_fields_in_study_lines_nullable.php
- resources/views/user/update_tenant_status.blade.php
- resources/views/components/input/checkbox.blade.php
- database/migrations/2025_07_11_184933_make_phone_number_nullable.php
- resources/views/auth/application/files.blade.php
- resources/views/components/input/textarea.blade.php
- resources/views/components/input/text.blade.php
- app/Http/Controllers/Auth/ApplicationController.php
- resources/views/user/study-line-selector.blade.php
- resources/views/utils/user/profile-picture.blade.php
- resources/views/user/alfonso-language-exams.blade.php
- app/Utils/HasRoles.php
- tests/Unit/AlfonsoTest.php
- resources/views/user/alfonso.blade.php
- app/Utils/ApplicationHandler.php
- resources/views/user/personal-information.blade.php
- resources/views/auth/application/questions.blade.php
- app/Models/Application.php
- tests/Feature/ApplicationTest.php
- app/Policies/UserPolicy.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: horcsinbalint
PR: EotvosCollegium/mars#743
File: app/Http/Controllers/Auth/AdmissionController.php:237-237
Timestamp: 2025-07-08T22:59:53.901Z
Learning: When horcsinbalint defers issues for "human reviewers to check later," he's making a conscious decision to prioritize the current PR over fixing secondary issues, and this should be acknowledged rather than being insistent about immediate fixes.
🧬 Code Graph Analysis (1)
app/Http/Controllers/Secretariat/UserController.php (9)
app/Models/EducationalInformation.php (4)
user
(83-86)EducationalInformation
(65-240)studyLines
(91-94)languageExams
(100-103)app/helpers.php (1)
user
(15-23)app/Models/RoleUser.php (3)
user
(62-65)role
(70-73)object
(54-57)app/Http/Controllers/StudentsCouncil/EpistolaController.php (1)
delete
(59-64)app/Models/User.php (8)
isCollegist
(753-770)roles
(218-222)isTenant
(849-852)personalInformation
(228-231)internetAccess
(370-373)workshops
(264-267)faculties
(305-308)educationalInformation
(237-240)app/Models/Role.php (1)
alumni
(293-296)app/Rules/SameOrUnique.php (1)
SameOrUnique
(10-63)app/Models/LanguageExam.php (1)
educationalInformation
(60-63)app/Http/Controllers/Secretariat/SemesterEvaluationController.php (1)
store
(162-246)
🔇 Additional comments (22)
resources/views/user/educational-information.blade.php (7)
3-19
: Well-structured permission-based messaging system.The conditional rendering of informational messages based on user permissions is implemented correctly. The distinction between full edit restrictions and partial edit restrictions provides clear user feedback whilst maintaining consistent email contact information across both scenarios.
28-32
: Proper implementation of dynamic form attributes.The high school input field correctly incorporates permission-based disabled states and required field indicators. The use of
user()->cannot('editStaticEducationalInformation', $user)
aligns with the authorization policy changes.
36-46
: Appropriate conditional logic for year fields.The year of acceptance field properly uses
$user->isCollegist(alumni: true) || isset($user->application)
for the required attribute, which correctly handles both collegist users and application scenarios.
71-84
: Consistent permission implementation for faculty selection.The faculty checkbox section correctly implements permission-based disabling and includes proper required field indicators. The use of
user()->cannot('edit', $user)
is consistent with the authorization model.
87-103
: Appropriate conditional rendering for workshop selection.The workshop section correctly uses
!isset($application)
to hide workshops during application processes and applieseditStaticEducationalInformation
permission checks appropriately.
105-111
: Proper authorization for study line management.The study line section correctly uses
@can('edit', $user)
to control access to the add study line button, ensuring users without edit permissions cannot modify this data.
117-133
: Consistent permission handling for textarea fields and save button.The research topics and extra information textareas, along with the save button, correctly implement permission-based disabling and visibility controls. The consistent use of
user()->cannot('edit', $user)
maintains proper authorization boundaries.app/Http/Controllers/Secretariat/UserController.php (15)
60-60
: Properly elevated authorization requirement.The change from 'view' to 'edit' authorization for profile picture management is correct and aligns with the principle of least privilege. Users should only modify profile pictures if they have edit permissions.
85-85
: Consistent authorization enhancement.The authorization check for deleting profile pictures correctly uses 'edit' permission, maintaining consistency with the store method and proper security boundaries.
105-130
: Excellent implementation of dynamic validation logic.The conditional validation rule building based on user permissions is well-structured. The use of
user()->can('editStaticPersonalInformation', $user)
properly restricts validation of sensitive fields whilst allowing other fields to be validated for all users with edit permissions.
142-145
: Proper array key existence check.The use of
array_key_exists('tenant_until', $data)
correctly handles the conditional update of tenant information, ensuring the field is only processed when present in the validated data.
159-192
: Well-structured dynamic validation for educational information.The educational information validation logic correctly separates static fields (requiring special permissions) from dynamic fields (requiring only edit permissions). The progressive building of validation rules ensures proper authorization whilst maintaining functionality.
213-244
: Robust transaction handling for data consistency.The use of DB transactions for educational information updates is excellent practice, ensuring data integrity across multiple related operations. The proper handling of study lines with null coalescing operators prevents undefined index errors.
222-228
: Appropriate conditional data synchronization.The conditional synchronization of workshop and faculty data using
array_key_exists
ensures that these relationships are only updated when the corresponding data is present in the request, preventing unintended data removal.
230-242
: Proper study line management with null safety.The study line creation logic correctly handles potential null values using the null coalescing operator, preventing undefined index errors whilst maintaining data integrity.
246-250
: User-friendly error handling with appropriate messaging.The conditional return logic provides clear feedback to users about Neptun code conflicts whilst allowing successful updates to proceed normally. The error message appropriately suggests contacting administrators for resolution.
263-271
: Appropriate authorization for Alfonso status updates.The use of
editStaticEducationalInformation
permission for Alfonso status updates is correct, as this information is considered static educational data. The simplified validation approach is appropriate for this specific use case.
286-304
: Consistent permission enforcement for language exam management.The language exam upload method correctly uses 'edit' permission and validates file uploads appropriately. The direct use of validated data in the creation method is clean and secure.
318-318
: Proper authorization for language exam deletion.The 'edit' permission requirement for deleting language exams is appropriate and maintains consistency with the upload functionality.
341-349
: Appropriate authorization and validation for tenant updates.The tenant until date update correctly uses 'edit' permission and validates the date range appropriately. The synchronization with internet access dates maintains data consistency.
422-422
: Consistent authorization pattern for role management.The use of
$this->authorize()
for role addition maintains consistency with Laravel's authorization patterns and properly enforces permission checks.
445-445
: Proper authorization for role removal.The authorization check for role removal correctly uses the same pattern as role addition, ensuring consistent permission enforcement across role management operations.
This commit also disables editing for applicants who have already submitted their application / when the application deadline has passed. It also disables some educational information that rarely changes for a user.
0f26c99
to
ddae735
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/Models/Application.php (1)
356-360
: Consistent implementation pattern with potential access concern.The educational information validation follows the same pattern as personal information. The same concern applies here regarding array versus object property access for the
$educationalInformation
model instance.
🧹 Nitpick comments (1)
app/Models/StudyLine.php (1)
15-15
: Consider updating PHPDoc to reflect nullable name property.The documentation declares
@property string $name
, but given the database migration has made this field nullable, it would be more accurate to document it as@property string|null $name
for consistency.- * @property string $name + * @property string|null $name
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
app/Http/Controllers/Auth/ApplicationController.php
(2 hunks)app/Http/Controllers/Secretariat/UserController.php
(14 hunks)app/Livewire/ParentChildForm.php
(1 hunks)app/Models/Application.php
(1 hunks)app/Models/EducationalInformation.php
(1 hunks)app/Models/StudyLine.php
(1 hunks)app/Policies/UserPolicy.php
(2 hunks)app/Utils/ApplicationHandler.php
(2 hunks)app/Utils/HasRoles.php
(1 hunks)database/migrations/2025_07_11_184933_make_phone_number_nullable.php
(1 hunks)database/migrations/2025_07_11_202220_make_fields_in_study_lines_nullable.php
(1 hunks)resources/lang/en/user.php
(2 hunks)resources/lang/hu/user.php
(2 hunks)resources/views/auth/application/app.blade.php
(1 hunks)resources/views/auth/application/files.blade.php
(1 hunks)resources/views/auth/application/questions.blade.php
(6 hunks)resources/views/components/input/checkbox.blade.php
(1 hunks)resources/views/components/input/datepicker.blade.php
(2 hunks)resources/views/components/input/file.blade.php
(1 hunks)resources/views/components/input/select.blade.php
(1 hunks)resources/views/components/input/text.blade.php
(1 hunks)resources/views/components/input/textarea.blade.php
(1 hunks)resources/views/components/input/timepicker.blade.php
(1 hunks)resources/views/livewire/parent-child-form.blade.php
(1 hunks)resources/views/user/alfonso-language-exams.blade.php
(1 hunks)resources/views/user/alfonso.blade.php
(1 hunks)resources/views/user/educational-information.blade.php
(3 hunks)resources/views/user/personal-information.blade.php
(1 hunks)resources/views/user/study-line-selector.blade.php
(1 hunks)resources/views/user/update_tenant_status.blade.php
(1 hunks)resources/views/utils/user/profile-picture.blade.php
(3 hunks)tests/Feature/ApplicationTest.php
(6 hunks)tests/Feature/UserTest.php
(0 hunks)tests/Unit/AlfonsoTest.php
(1 hunks)
💤 Files with no reviewable changes (1)
- tests/Feature/UserTest.php
✅ Files skipped from review due to trivial changes (2)
- resources/views/user/update_tenant_status.blade.php
- resources/views/components/input/checkbox.blade.php
🚧 Files skipped from review as they are similar to previous changes (27)
- resources/views/auth/application/app.blade.php
- resources/views/components/input/file.blade.php
- resources/views/components/input/select.blade.php
- tests/Unit/AlfonsoTest.php
- resources/views/auth/application/files.blade.php
- resources/views/components/input/datepicker.blade.php
- resources/views/components/input/timepicker.blade.php
- database/migrations/2025_07_11_202220_make_fields_in_study_lines_nullable.php
- app/Utils/HasRoles.php
- resources/lang/en/user.php
- database/migrations/2025_07_11_184933_make_phone_number_nullable.php
- resources/views/components/input/text.blade.php
- resources/views/user/alfonso-language-exams.blade.php
- resources/views/utils/user/profile-picture.blade.php
- app/Models/EducationalInformation.php
- resources/views/components/input/textarea.blade.php
- resources/views/user/alfonso.blade.php
- resources/views/user/study-line-selector.blade.php
- app/Http/Controllers/Auth/ApplicationController.php
- resources/views/auth/application/questions.blade.php
- resources/lang/hu/user.php
- app/Policies/UserPolicy.php
- resources/views/user/personal-information.blade.php
- resources/views/user/educational-information.blade.php
- tests/Feature/ApplicationTest.php
- app/Utils/ApplicationHandler.php
- app/Http/Controllers/Secretariat/UserController.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: horcsinbalint
PR: EotvosCollegium/mars#743
File: app/Http/Controllers/Auth/AdmissionController.php:237-237
Timestamp: 2025-07-08T22:59:53.901Z
Learning: When horcsinbalint defers issues for "human reviewers to check later," he's making a conscious decision to prioritize the current PR over fixing secondary issues, and this should be acknowledged rather than being insistent about immediate fixes.
🧬 Code Graph Analysis (1)
app/Models/Application.php (3)
app/Models/EducationalInformation.php (3)
user
(83-86)studyLines
(91-94)alfonsoExempted
(158-162)app/helpers.php (1)
user
(15-23)app/Models/User.php (3)
personalInformation
(228-231)educationalInformation
(237-240)faculties
(305-308)
🔇 Additional comments (9)
app/Models/StudyLine.php (1)
101-101
: Excellent defensive programming approach for nullable field handling.The null coalescing operator ensures the method maintains its string return type contract whilst gracefully handling the newly nullable
name
database column. This is a sound approach that prevents potential null return values.app/Models/Application.php (6)
344-349
: Excellent implementation of granular user validation.The explicit validation of user name and email represents a significant improvement over previous broad checks. The implementation correctly validates these fundamental user attributes whilst maintaining proper null safety.
362-364
: Proper relationship validation implementation.The faculty count validation correctly utilises the Eloquent relationship method to verify that at least one faculty has been selected. This represents sound Laravel practice.
367-384
: Comprehensive study line validation with proper defensive programming.The implementation demonstrates excellent defensive programming practices by first checking for the existence of educational information and study lines before iterating. The individual field validation for each study line (name, type, training_code, start) provides the granular validation mentioned in the PR objectives.
385-392
: Excellent conditional validation with proper exemption handling.The ALFONSÓ language validation correctly implements conditional logic that only applies when
year_of_acceptance
is set and the user is not exempted. The utilisation of thealfonsoExempted()
method demonstrates proper separation of concerns and maintainable code architecture.
394-414
: Enhanced categorisation improves user experience.The addition of the "Szakmai és motivációs kérdések:" prefix to these validation messages significantly improves the user experience by clearly categorising the missing data. This aligns perfectly with the PR's objective of providing more detailed validation feedback.
416-418
: Proper positioning of file validation requirement.Moving the file count validation to the conclusion of the method ensures that users receive comprehensive feedback about all missing data types before addressing file upload requirements. This represents sound UX design principles.
app/Livewire/ParentChildForm.php (1)
18-18
: Well-implemented attribute customisation property.I say, this is a rather sensible addition that provides excellent flexibility for input customisation. The default value is quite appropriate, and the documentation is spot on.
resources/views/livewire/parent-child-form.blade.php (1)
21-29
: Critical security concern: Unescaped attribute output poses XSS risk.I must say, whilst the restructuring is rather clever, there's a rather serious security vulnerability here. The template is outputting both attribute names and values without proper escaping, which could lead to XSS attacks if malicious content is provided via
$inputAttributes
.Apply this diff to properly escape the attribute output:
- <input - class="validate parent-child parent-child-input" - @foreach ($inputAttributes as $attrName => $attrValue) - {{ $attrName }}="{{ $attrValue }}" - @endforeach - - name="{{ $name }}[]" - wire:model.live="items.{{ $index }}" - > + <input + class="validate parent-child parent-child-input" + @foreach ($inputAttributes as $attrName => $attrValue) + {!! e($attrName) !!}="{!! e($attrValue) !!}" + @endforeach + + name="{{ $name }}[]" + wire:model.live="items.{{ $index }}" + >Alternatively, consider using Laravel's
Html::attributes()
helper if available, or implement server-side validation to ensure only safe attribute names and values are permitted.Likely an incorrect or invalid review comment.
foreach (['place_of_birth', 'date_of_birth', 'mothers_name', 'phone_number', | ||
'country', 'county', 'zip_code', 'city', 'street_and_number'] as $personal_info_field) { | ||
if (!isset($personalInformation) || !isset($personalInformation[$personal_info_field])) { | ||
$missingData[] = "Személyes adat: ".strtolower(__('user.'.$personal_info_field)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify array access pattern for personal information fields.
The validation logic appears sound, however, I must raise a concern regarding the array access pattern. The condition !isset($personalInformation[$personal_info_field])
suggests that $personalInformation
is being treated as an array, yet the model relationship indicates it should be a PersonalInformation
object instance.
Please run this verification script to confirm the expected data structure:
🏁 Script executed:
#!/bin/bash
# Description: Verify how PersonalInformation model properties are accessed in the codebase
# Expected: Should reveal whether object property access or array access is the correct pattern
echo "=== Searching for PersonalInformation property access patterns ==="
rg -A 3 -B 1 'personalInformation\->' --type php
echo "=== Searching for PersonalInformation array access patterns ==="
rg -A 3 -B 1 'personalInformation\[' --type php
echo "=== Checking PersonalInformation model definition ==="
cat app/Models/PersonalInformation.php | head -50
Length of output: 6992
Please replace array-style access with object property access on the PersonalInformation model
To keep consistency with Eloquent models (and the rest of the codebase), PersonalInformation
should be treated as an object, not an array. In app/Models/Application.php around lines 350–355, update the check to use property access:
• File: app/Models/Application.php
Lines: ~350–355
Diff:
- foreach (['place_of_birth', 'date_of_birth', 'mothers_name', 'phone_number',
- 'country', 'county', 'zip_code', 'city', 'street_and_number'] as $personal_info_field) {
- if (!isset($personalInformation) || !isset($personalInformation[$personal_info_field])) {
- $missingData[] = "Személyes adat: ".strtolower(__('user.'.$personal_info_field));
- }
- }
+ foreach (['place_of_birth', 'date_of_birth', 'mothers_name', 'phone_number',
+ 'country', 'county', 'zip_code', 'city', 'street_and_number'] as $personal_info_field) {
+ if (! isset($personalInformation)
+ || ! isset($personalInformation->{$personal_info_field})
+ ) {
+ $missingData[] = "Személyes adat: "
+ .strtolower(__('user.'.$personal_info_field));
+ }
+ }
This ensures you’re checking the model’s attributes via object properties rather than array offsets.
📝 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.
foreach (['place_of_birth', 'date_of_birth', 'mothers_name', 'phone_number', | |
'country', 'county', 'zip_code', 'city', 'street_and_number'] as $personal_info_field) { | |
if (!isset($personalInformation) || !isset($personalInformation[$personal_info_field])) { | |
$missingData[] = "Személyes adat: ".strtolower(__('user.'.$personal_info_field)); | |
} | |
} | |
foreach (['place_of_birth', 'date_of_birth', 'mothers_name', 'phone_number', | |
'country', 'county', 'zip_code', 'city', 'street_and_number'] as $personal_info_field) { | |
if (! isset($personalInformation) | |
|| ! isset($personalInformation->{$personal_info_field}) | |
) { | |
$missingData[] = "Személyes adat: " | |
.strtolower(__('user.'.$personal_info_field)); | |
} | |
} |
🤖 Prompt for AI Agents
In app/Models/Application.php around lines 350 to 355, the code currently
accesses the PersonalInformation model attributes using array syntax, which is
inconsistent with Eloquent model usage. Update the condition to check properties
using object property access (e.g., $personalInformation->place_of_birth)
instead of array keys. This change aligns with the rest of the codebase and
proper Eloquent model conventions.
This could theoretically allow applicants (but not verified users) to change their acceptance year. However, we supervise applications manually and they'd have to bypass the disabled input field on the front end, so this is not a huge issue.
Description
Now applicants are able to save partially filled forms during the application process and the presence of those values are only checked when they submit their entire application.
Furthermore, applicants are no longer able to edit data once they have submitted their application.
For editing userdata, being able to view it is no longer enough, the UserPolicy has been revised a bit (only the user, secretariat, student council leaders and sysadmins can edit data from now on). Users should not be allowed to edit some of their data (like workshop list, date of birth).
Related Issue
Closes #234