Skip to content

[WRSAT-488] Make email logic safer#197

Merged
cram-cfa merged 3 commits intomainfrom
WRSAT-488
Apr 3, 2026
Merged

[WRSAT-488] Make email logic safer#197
cram-cfa merged 3 commits intomainfrom
WRSAT-488

Conversation

@cram-cfa
Copy link
Copy Markdown
Contributor

@cram-cfa cram-cfa commented Apr 3, 2026

No description provided.

@jenny-heath jenny-heath temporarily deployed to wrsat-wrsat-488-zehbz6errqud9t April 3, 2026 19:54 Inactive
@county_data = LocationData::Counties.get(@screener.state, @screener.county)
@state = @screener.state
@county = @screener.county
attachments.inline["gbh_email_header.png"] = File.binread(Rails.root.join("app/assets/images/gbh_email_header.png"))
Copy link
Copy Markdown
Collaborator

@anisharamnani anisharamnani Apr 3, 2026

Choose a reason for hiding this comment

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

i’m not totally sure why these need to be instance variables since they’re accessible by the @screener

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed them, and just created the state and county in the two erb files to make things easier to read!

Copy link
Copy Markdown
Collaborator

@anisharamnani anisharamnani left a comment

Choose a reason for hiding this comment

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

i have a minor thing about creating an instance variable, when i think that the instance @screener variable gives you access to the methods you need but i could be wrong.

@jenny-heath jenny-heath temporarily deployed to wrsat-wrsat-488-zehbz6errqud9t April 3, 2026 21:32 Inactive
@jenny-heath jenny-heath temporarily deployed to wrsat-wrsat-488-zehbz6errqud9t April 3, 2026 21:33 Inactive
<% state = @screener.state %>
<% county = @screener.county %>
<% website = LocationData::Counties.website_for(state, county) %>
<% email = LocationData::Counties.email_for(state, county) %>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These are just so make it easier to read, since the values are reused in a couple places within the file.

@cram-cfa cram-cfa merged commit ec66d20 into main Apr 3, 2026
9 checks passed
@cram-cfa cram-cfa deleted the WRSAT-488 branch April 3, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants