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

Refactor URL hardcoding #422

Closed
5 tasks done
danmash opened this issue Oct 22, 2022 · 2 comments · Fixed by #460
Closed
5 tasks done

Refactor URL hardcoding #422

danmash opened this issue Oct 22, 2022 · 2 comments · Fixed by #460
Assignees

Comments

@danmash
Copy link
Member

danmash commented Oct 22, 2022

What to refactor?

There are cases in the codebase when we are using hardcoding URLs to the project web app:

  • app.models.PasswordResetLink.reset_url
  • app/sendgrid/templates/reset_password.py template and usage
  • app/sendgrid/templates/user_b_shared_email.py template and usage
  • app/sendgrid/templates/welcome_email.py template and usage

Refactor reason

The refactoring is needed because we need the ability to use different URLs per environment:

Suggested solution

  • Update all configs from config.py to use the env variable with URL (this solution doesn't suitable for many production envs I suppose, but it's ok for now)

Acceptance criteria

  • missing ENV variable shouldn't lead to an error, default config value should be prod

Tests

  • manual tests
  • unit tests with mocking for the class method
  • integration tests with mocking for emails
@brianpeiris
Copy link
Member

I can look into this

@danmash danmash removed the good first issue Good for newcomers label Nov 21, 2022
danmash added a commit that referenced this issue Nov 28, 2022
@danmash
Copy link
Member Author

danmash commented Nov 28, 2022

I've added env variable to staging and it works fine
Screenshot 2022-11-28 at 14 38 59

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants