-
Notifications
You must be signed in to change notification settings - Fork 1
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
clean(ZMS-3377): added new migration to change default text #712
base: next
Are you sure you want to change the base?
Conversation
WalkthroughThe changes involve updates to the appointment form's user interface in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Form
participant Database
User->>Form: Open appointment form
Form->>Database: Fetch preferences
Database-->>Form: Return preferences
Form->>User: Display form with dynamic labels
User->>Form: Fill out form
Form->>Database: Save custom text field label
Database-->>Form: Confirm save
Form->>User: Show confirmation message
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
|
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
🧹 Outside diff range and nitpick comments (3)
zmsdb/migrations/91732870750-custom-textfiled-label.sql (2)
1-3
: Consider adding data validationIt would be beneficial to add a verification step to ensure the update was successful:
BEGIN; UPDATE `standort` SET `custom_text_field_label` = 'Zusätzliche Bemerkungen (für Bürger*innen sichtbar)' WHERE `custom_text_field_label` = '' OR `custom_text_field_label` IS NULL OR `custom_text_field_label` = 'Zusätzliche Bemerkungen'; + -- Verify the update + SELECT COUNT(*) as remaining_to_update + FROM `standort` + WHERE `custom_text_field_label` = '' OR `custom_text_field_label` IS NULL OR `custom_text_field_label` = 'Zusätzliche Bemerkungen'; COMMIT;
1-3
: Fix typo in migration filenameThe migration filename contains a typo: "textfiled" should be "textfield".
Consider renaming the file to:
91732870750-custom-textfield-label.sql
zmsadmin/custom_templates/block/appointment/form.twig (1)
Line range hint
271-285
: Fix duplicate accesskey and add default labelTwo issues need attention:
- The
accesskey="a"
is duplicated (also used in the email field above), which can cause accessibility issues.- Consider adding a default label in case the preference is not set.
Apply this diff to fix the issues:
- "label": scope.preferences.client.customTextfieldLabel, + "label": scope.preferences.client.customTextfieldLabel|default('Zusätzliche Bemerkungen'), "required": selectedProcess.preferences.client.customTextfieldRequired|default(scope.preferences.client.customTextfieldRequired), "errors": formData.customTextfield.messages, "checked": scope.preferences.client.customTextfieldActivated, }, [{ "type": "input", "parameter": { "name": "customTextfield", "value": selectedProcess.customTextfield, - "accesskey":"a" + "accesskey":"u" } }]
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
zmsadmin/custom_templates/block/appointment/form.twig
(1 hunks)zmsdb/migrations/91732870750-custom-textfiled-label.sql
(1 hunks)
🔇 Additional comments (2)
zmsdb/migrations/91732870750-custom-textfiled-label.sql (1)
1-3
: Verify impact on existing appointments
The change in label text might affect existing appointments or the user interface.
✅ Verification successful
Let me try another search pattern to gather more information about how this field is used in the codebase.
Migration is safe to proceed
Based on the codebase analysis, the label change is safe because:
- The label text is dynamically rendered from the database value in all UI templates
- The system uses the
custom_text_field_label
column value through proper database mappings (Scope.php
) - The change only updates empty or specific text values, preserving any other custom labels that might exist
The migration updates the label text in a controlled manner:
- Empty labels (
''
) - NULL values
- Exact match of old text ('Zusätzliche Bemerkungen')
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any UI templates or code that might reference the old label text
rg -l "Zusätzliche Bemerkungen" --type php --type html --type twig
# Check for any potential customizations in the standort table
echo "Sample query to check current distinct values:"
echo "SELECT DISTINCT custom_text_field_label FROM standort WHERE custom_text_field_label NOT IN ('', 'Zusätzliche Bemerkungen') AND custom_text_field_label IS NOT NULL;"
Length of output: 520
Script:
#!/bin/bash
# Search for references to the column name in PHP files
rg -i "custom_text_field_label" --type php -C 2
# Search for form fields or template variables that might use this label
rg -i "textfield.*label|label.*textfield" --type php --type html --type twig -C 2
Length of output: 11344
zmsadmin/custom_templates/block/appointment/form.twig (1)
271-271
: Verify database schema alignment
Let's ensure that the customTextfieldLabel
preference aligns with the database schema mentioned in the migration.
✅ Verification successful
Database schema and template usage are properly aligned
The verification shows proper alignment between the database schema and template usage:
- The database schema in migrations properly defines
custom_text_field_label
in thestandort
table - The template's usage of
scope.preferences.client.customTextfieldLabel
is consistent across multiple template files - Default values and updates for the label are properly handled in migrations
- The feature is consistently integrated across various views (queue, process, appointment forms)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the preference key exists in relevant SQL files and verify its usage
# Look for the preference key in SQL files
echo "Checking SQL files for customTextfieldLabel..."
rg -t sql "customTextfieldLabel|custom_text_field_label" -A 5 -B 5
# Look for other usages of this preference in templates
echo "Checking other template files for consistency..."
rg "preferences\.client\.customTextfieldLabel" -t html -t twig -A 2 -B 2
Length of output: 9877
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Description
Short description or comments
Reference
Issues #XXX
Summary by CodeRabbit
New Features
Bug Fixes
Chores