Skip to content

Conversation

@sraby
Copy link
Member

@sraby sraby commented Oct 19, 2020

This PR makes some minor tweaks to our Letter of Complaint content, as requested during a content audit.

@sraby
Copy link
Member Author

sraby commented Oct 19, 2020

@toolness since these are quick content changes, I'm gonna go ahead and merge myself. I'll also make sure to merge master into our #1649 PR and update conflicts.


assert second.area_complained_of_mc == hp.AreaComplainedOfMC.PUBLIC_AREA
assert second.which_room_mc.value == "Public areas" # type: ignore
assert second.which_room_mc.value == "Building-wide" # type: ignore
Copy link
Member Author

Choose a reason for hiding this comment

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

Hey @toolness! Noticed that me changing the names of the issue areas broke this HPA back-end test. I fixed it here but just wanted to make sure there weren't any other HP-related consequences of this copy change. Lmk!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah right, there shouldn't be any, since outside of tests, all string comparisons are done against the "code" (e.g. `PUBLIC_AREAS") rather than the user-facing string! NOICE

@sraby sraby requested a review from toolness October 19, 2020 19:42
@sraby
Copy link
Member Author

sraby commented Oct 19, 2020

@toolness on the second thought, lemme just get your eyes on this quick— see my comment above ^^

Comment on lines +31 to +32
HOME: "Home-wide",
PUBLIC_AREAS: "Building-wide",
Copy link
Member Author

Choose a reason for hiding this comment

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

One of the requests was to reorder these sections, hence the change here

Copy link
Collaborator

@toolness toolness left a comment

Choose a reason for hiding this comment

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

This looks good to me! Just to confirm, all the changes you made don't need to be internationalized or translated prior to pushing to production right?


assert second.area_complained_of_mc == hp.AreaComplainedOfMC.PUBLIC_AREA
assert second.which_room_mc.value == "Public areas" # type: ignore
assert second.which_room_mc.value == "Building-wide" # type: ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah right, there shouldn't be any, since outside of tests, all string comparisons are done against the "code" (e.g. `PUBLIC_AREAS") rather than the user-facing string! NOICE

@sraby
Copy link
Member Author

sraby commented Oct 19, 2020

@toolness yep! They're only LOC (and bc of the issues, EHP) changes, and since those tools aren't i18n enabled yet, we can push this.

@sraby sraby merged commit ec602a5 into master Oct 19, 2020
@sraby sraby deleted the loc-changes branch October 19, 2020 19:59
toolness added a commit that referenced this pull request Oct 21, 2020
#1722 made some trivial re-orderings to how we list issue areas, but due to Django's migration logic sillyness (which I think has actually just been fixed in Django's main branch) it requires a completely trivial migration. This is it.
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