Skip to content
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

[#139] Enhance client and volunteer signup forms #140

Merged
merged 8 commits into from
Apr 19, 2024

Conversation

delano
Copy link
Contributor

@delano delano commented Apr 19, 2024

Working on #134 #139

This pull request includes enhancements to the client and volunteer signup forms. It adds phone number help text on the volunteer form, clarifies the email confirmation and staff review process for volunteers, and references the privacy notice and terms on both forms. It also unifies the submit button labels for consistency.

These changes strengthen privacy protections and provide clearer expectations for new users.

delano added 8 commits April 17, 2024 20:18
Updated form fields to consistently label required fields. Use validation template to standardize ToS placement. Minor copy tweaks for clarity.

### Changes

- Labeled all required fields with "*"
- Added help text for optional phone field
- Removed unused SMS checkbox
- Moved ToS/footer content to shared validation template
Recursive Django commands to use dotenv and pnpm consistently.
- Add phone number help text on volunteer form
- Clarify email confirmation and staff review process for volunteers
- Reference privacy notice and terms on both forms
- Unify submit button labels for consistency

These changes strengthen privacy protections and provide clearer expectations for new users.
@delano delano self-assigned this Apr 19, 2024
Copy link
Contributor

qodo-merge-pro bot commented Apr 19, 2024

CI Failure Feedback

(Checks updated until commit 124a983)

Action: install-and-cache (ubuntu-latest, 3.11, 5)

Failed stage: Install django 5 [❌]

Failure summary:

The action failed because it could not find a matching version of the package Django. This issue
occurred during the process of recreating the virtual environment found in .venv, which was deemed
to be broken. The failure indicates a problem with package dependency resolution, possibly due to
specifying a version of Django that does not exist or is not available in the configured package
repository.

Relevant error logs:
1:  ##[group]Operating System
2:  Ubuntu
...

624:  Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.9/x64
625:  LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.9/x64/lib
626:  VENV: .venv/bin/activate
627:  VIRTUAL_ENV: .venv
628:  ##[endgroup]
629:  The virtual environment found in .venv seems to be broken.
630:  Recreating virtualenv afb requests in /home/runner/work/afb-requests/afb-requests/.venv
631:  Could not find a matching version of package Django
632:  ##[error]Process completed with exit code 1.

✨ CI feedback usage guide:

The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
The tool analyzes the failed checks and provides several feedbacks:

  • Failed stage
  • Failed test name
  • Failure summary
  • Relevant error logs

In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

/checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"

where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

Configuration options

  • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
  • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
  • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
  • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
  • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

See more information about the checks tool in the docs.

Copy link
Contributor

PR Review

⏱️ Estimated effort to review [1-5]

3, because the PR involves multiple files across different parts of the application, including frontend Vue components and backend settings. The changes impact both the UI and the underlying logic, requiring a thorough review to ensure consistency and functionality.

🧪 Relevant tests

No

🔍 Possible issues

Possible Bug: The removal of the phone number requirement in apps/ui/pages/welcome/clients.vue without a corresponding backend validation change could lead to issues if the backend still expects a phone number.

Consistency Issue: The change from "Privacy Policy" to "Privacy Notice" across various files needs to be reflected everywhere in the application to maintain consistency. If there are any instances where "Privacy Policy" is still used, it could confuse users.

🔒 Security concerns

No

@@ -33,13 +33,8 @@ const fields = [
label: "Phone",
placeholder: "Your phone number",
icon: "i-heroicons-phone",
help: "If you provide a phone number, we will use it to coordinate your delivery.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding backend validation for optional phone numbers to ensure data integrity and avoid potential issues with data processing or communication features that rely on phone numbers. [important]

label: 'Can we text you at this number?',
type: 'checkbox',
value: 'yes'
help: "We require a phone number for verification. You will also be able to receive SMS notifications at this number if you choose to.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the removal of the 'can_sms' checkbox is reflected in the backend logic, particularly if the backend uses this field to determine whether SMS messages can be sent to the user. [important]

@@ -17,8 +17,9 @@
"typecheck": "nuxt typecheck apps/ui",
"prettier": "prettier --write \"apps/ui/src/**/*.{ts,tsx}\"",
"test": "jest apps/ui",
"django": "python apps/api/manage.py",
"django:run": "python apps/api/manage.py runserver_plus",
"django": "dotenv python apps/api/manage.py",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the change from direct python commands to using pnpm for Django management commands is clearly documented in the project's README or developer documentation to avoid confusion. [medium]

Copy link
Contributor

qodo-merge-pro bot commented Apr 19, 2024

PR Code Suggestions

CategorySuggestions                                                                                                                                                       
Enhancement
Suggest using consistent terminology for privacy policies.

Consider using a more generic term like "Privacy Policy" instead of "Privacy Notice" to
maintain consistency across different sections of the application unless there's a
specific reason for the distinction. This change should be reflected in both the checkbox
text for accepting terms and the actual title of the privacy policy page.

apps/ui/components/requests/FoodRequestForm.vue [368]

-text: "I have read, accepted, and agreed to the Terms and Conditions and Privacy Notice."
+text: "I have read, accepted, and agreed to the Terms and Conditions and Privacy Policy."
 
Simplify Django-related script commands for consistency.

For the new Django-related scripts added to package.json, consider using a single
environment command for Django operations to simplify the command structure. This can help
in maintaining consistency and ease of use.

package.json [20-22]

 "django": "dotenv python apps/api/manage.py",
-"django:shell": "pnpm django shell_plus",
-"django:run": "pnpm django runserver_plus",
+"django:shell": "dotenv python apps/api/manage.py shell_plus",
+"django:run": "dotenv python apps/api/manage.py runserver_plus",
 
Possible issue
Ensure the code input field is interactive for user input.

The UInput component for the code input field is marked as disabled. If this field is
intended for users to enter a code, it should not be disabled. Ensure that the field is
interactive so users can input their verification code.

apps/ui/pages/login/check.vue [113]

-<UInput v-model="code" name="code" type="text" color="primary" variant="outline" disabled placeholder="Code" label="Code" size="xl" class="w-32 mx-auto" required />
+<UInput v-model="code" name="code" type="text" color="primary" variant="outline" placeholder="Code" label="Code" size="xl" class="w-32 mx-auto" required />
 
Ensure phone number validation aligns with field requirements.

The validate function no longer checks for the presence of a phone number, which may be an
oversight if the phone number is still required. If the phone number is optional, ensure
that the UI clearly indicates this to the user. If it's required, add back the validation
check for the phone number.

apps/ui/pages/welcome/clients.vue [52-54]

 if (!validateEmail(state.email)) {
   errors.push({ path: "email", message: "Please enter a valid email address" });
 }
+if (!state.phone) {
+  errors.push({ path: "phone", message: "Phone number is required" });
+}
 
Accessibility
Add a visible label to the email input field for better accessibility.

The placeholder text for the email input field has been changed to "Your email address"
without a corresponding label. Consider adding a visible label for accessibility reasons,
as placeholders alone may not be sufficient for all users, especially those using screen
readers.

apps/ui/pages/login/index.vue [39]

+label: "Email Address"
 placeholder: "Your email address"
 

✨ Improve tool usage guide:

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

  • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

@delano delano changed the title Enhance client and volunteer signup forms [#139] Enhance client and volunteer signup forms Apr 19, 2024
@delano delano merged commit 1f334c6 into 134-updates-from-privacy-review Apr 19, 2024
0 of 2 checks passed
@delano delano deleted the delano/139-signup-login branch April 19, 2024 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant