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.

Sign up for GitHub

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

Jump to bottom

P20-903: Display U13 Warning on Section Creation to USA teachers #58688

Merged
merged 3 commits into from
May 17, 2024
Merged

P20-903: Display U13 Warning on Section Creation to USA teachers #58688

merged 3 commits into from
May 17, 2024

Conversation

artem-vavilov
Copy link
Contributor

Error

==============[0m[1000D[KERROR["test_returns_nil_if_student_us_state_is_\"??\"", "UserTest::UsStateCodeTest", 56.4121803099988]
 test_returns_nil_if_student_us_state_is_"??"#UserTest::UsStateCodeTest (56.41s)
Minitest::UnexpectedError:         Errno::EACCES: Permission denied @ rb_sysopen - /usr/local/lib/ruby/gems/3.0.0/gems/city-state-1.1.0/lib/db/states.us
            app/models/user.rb:2746:in `us_state_code'
            test/models/user_test.rb:18:in `block in <class:UsStateCodeTest>'
            test/testing/setup_all_and_teardown_all.rb:36:in `run'

Links

@artem-vavilov artem-vavilov requested a review from a team May 16, 2024 18:13
wilkie
wilkie reviewed May 16, 2024
View reviewed changes
dashboard/test/models/user_test.rb
test 'returns teacher school US state code when state is name' do
teacher = create(:teacher, school_info: create(:school_info, state: 'washington dc'))
assert_equal 'DC', teacher.us_state_code
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should cover the case where the school_info does not have a state recorded or has an unknown state ... maybe it is a school in Canada, Europe, etc. I think you handle that case in your implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically, cover the case where the teacher's state would be nil I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e187410

@artem-vavilov artem-vavilov requested a review from wilkie May 16, 2024 19:14
wilkie
wilkie approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@wilkie wilkie left a comment

Choose a reason for hiding this comment

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

Great work! 👍

@artem-vavilov artem-vavilov merged commit 348a49b into staging May 17, 2024
2 checks passed
@artem-vavilov artem-vavilov deleted the P20-903/display-u13-warning-for-usa-teachers branch May 17, 2024 17:15
This was referenced May 17, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wilkie wilkie wilkie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@artem-vavilov @wilkie