-
Notifications
You must be signed in to change notification settings - Fork 8
Sf fields #1632
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
base: main
Are you sure you want to change the base?
Conversation
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 aims to update Salesforce-related fields and adjust related functionality for user contact information.
- Removed assignments for user.is_kip and user.grant_tutor_access in update_user_contact_info.rb
- Commented out the ConfirmationMailer call in send_contact_info_confirmation.rb
- Updated the openstax_salesforce gem dependency in Gemfile
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
app/routines/update_user_contact_info.rb | Removed Salesforce field assignments that are no longer required |
app/routines/send_contact_info_confirmation.rb | Temporarily disabled sending confirmation emails by commenting out the mailer invocation |
Gemfile | Pinned the openstax_salesforce gem to a specific GitHub ref for a temporary solution |
Comments suppressed due to low confidence (2)
app/routines/update_user_contact_info.rb:119
- Confirm that the removal of the user.is_kip and user.grant_tutor_access assignments is fully intentional and that downstream logic does not rely on these fields. If these changes have broader implications, ensure that related documentation and tests are updated accordingly.
user.is_kip = sf_school&.is_kip || sf_school&.is_child_of_kip
app/routines/send_contact_info_confirmation.rb:19
- If disabling the ConfirmationMailer call is intentional, consider removing the commented code or adding a clear inline comment explaining the temporary nature of this change.
ConfirmationMailer.instructions(email_address: contact_info, send_pin: send_pin).deliver_later
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 updates the student signup flow and related Salesforce integration code. Key changes include:
- Removal of VCR cassette usage and associated setup in the student signup flow test.
- Modification of user contact info updates by removing assignments for is_kip and grant_tutor_access.
- Change in dependency loading in the Puma configuration and addition of the concurrent-ruby gem.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
spec/features/newflow/student_signup_flow_spec.rb | Removed VCR cassette usage and before(:all) block for SalesforceProxy setup |
spec/cassettes/Newflow/Students/student_signup_flow/sf_setup.yml | Removed unused cassette file |
config/puma.rb | Updated dotenv gem to use "dotenv/load" |
app/routines/update_user_contact_info.rb | Removed assignment of is_kip and grant_tutor_access fields |
Gemfile | Added concurrent-ruby gem |
Comments suppressed due to low confidence (2)
spec/features/newflow/student_signup_flow_spec.rb:5
- Removal of the VCR cassette setup may lead to tests executing real HTTP requests; ensure that external calls are appropriately stubbed or handled elsewhere to maintain reliable test behavior.
feature 'Student signup flow', js: true do
app/routines/update_user_contact_info.rb:119
- The removal of assignments for user.is_kip and user.grant_tutor_access might impact downstream logic that relies on these fields; please confirm that this change aligns with the updated Salesforce field mappings.
user.is_kip = sf_school&.is_kip || sf_school&.is_child_of_kip
No description provided.