Skip to content

#1790 - SMS opt-in to system imports with mobile numbers#3380

Merged
compwron merged 24 commits into
rubyforgood:mainfrom
harsohailB:feature/1790
Apr 26, 2022
Merged

#1790 - SMS opt-in to system imports with mobile numbers#3380
compwron merged 24 commits into
rubyforgood:mainfrom
harsohailB:feature/1790

Conversation

@harsohailB
Copy link
Copy Markdown
Collaborator

@harsohailB harsohailB commented Apr 18, 2022 •
edited by FireLemons
Loading

What github issue is this PR for, if any?

Resolves #1790

What changed, and why?

  • Write function to detect mobile numbers exist
  • Create opt-in modal
  • Pause import if opt-in required and show modal
  • Temporarily store CSV in local storage
  • Delete CSV from local storage upon successful import
  • Resume import upon confirming opt-in
  • Turn on SMS notifications for imported users

How will this affect user permissions?

  • Volunteer permissions: N/A
  • Supervisor permissions: N/A
  • Admin permissions: N/A

How is this tested? (please write tests!) 💖💪

  • Update import request tests
  • Update volunteer and supervisor importer tests
  • Add system tests for opt in modal

Screenshots please :)

image

@harsohailB harsohailB added zzz_archived: 📱 SMS work relating to SMS notifications #1017 codethechange code.the.change developers labels Apr 18, 2022
@harsohailB harsohailB self-assigned this Apr 18, 2022
@harsohailB harsohailB marked this pull request as draft April 18, 2022 01:11
@harsohailB harsohailB added the Help Wanted Looking for contributors label Apr 18, 2022
@harsohailB harsohailB marked this pull request as ready for review April 23, 2022 01:52
xihai01
xihai01 reviewed Apr 24, 2022
View reviewed changes
Comment thread app/controllers/imports_controller.rb Outdated
xihai01
xihai01 reviewed Apr 24, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@xihai01 xihai01 left a comment

Choose a reason for hiding this comment

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

lgtm

@harsohailB harsohailB requested a review from xihai01 April 24, 2022 18:37
xihai01
xihai01 approved these changes Apr 25, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@xihai01 xihai01 left a comment

Choose a reason for hiding this comment

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

wow awesome work. I think it looks good. Maybe the others will want to take a look too?

@7riumph
Copy link
Copy Markdown
Collaborator

7riumph commented Apr 25, 2022

wow awesome work. I think it looks good. Maybe the others will want to take a look too?

I agree, it looks great to me, though @compwron /a maintainer should definitely have a look since we’ve reached into javascript territory with the CSV file caching additions in app/javascript/src/import.js

@compwron compwron merged commit ce32d79 into rubyforgood:main Apr 26, 2022
@compwron
Copy link
Copy Markdown
Collaborator

compwron commented Apr 26, 2022

very very cool!!!

@harsohailB harsohailB mentioned this pull request May 22, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@compwron compwron compwron approved these changes

@7riumph 7riumph 7riumph approved these changes

@xihai01 xihai01 xihai01 approved these changes

@frankljin frankljin Awaiting requested review from frankljin

Assignees

@harsohailB harsohailB

Labels

codethechange code.the.change developers erb Touches ERB templates javascript Touches JavaScript code ruby Touches Ruby code 🧪 Tests Tests zzz_archived: 📱 SMS work relating to SMS notifications #1017

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

add SMS opt-in to system imports with mobile numbers

4 participants

@harsohailB @7riumph @compwron @xihai01