Conversation
ConlynPattison
changed the title
| # Setup | ||
| mock_result = Mock() | ||
| mock_result._asdict.return_value = student_row_data | ||
| mock_postgresql_db.execute.return_value.first.return_value = mock_result |
There was a problem hiding this comment.
This fixture is a bit odd; All the fixture does is return static data, but in the tests, they need to be loaded in the database object anyway (for each test). I understand having a custom fixture for this class of tests, but I feel like the data loading functionality should have either been part of the fixture or assigned to a helper function.
|
|
||
| assert result['first_gen'] is True | ||
|
|
||
| def test_fetch_student_info_with_first_generation_false(self, mock_postgresql_db, student_row_data): |
There was a problem hiding this comment.
This test could be merged with the above test (using parameterized tests), especially since the code is the same, only the values are changing.
|
|
||
| assert result['first_gen'] is False | ||
|
|
||
| def test_fetch_student_info_with_different_target_years(self, mock_postgresql_db, student_row_data): |
There was a problem hiding this comment.
On an initial pass, I assumed that these tests were redundant, but then I remembered the problems we had with re-factoring. While these tests are very unlikely to ever fail, I appreciate the added safeguards.
NickGuerrero
left a comment
NickGuerrero
left a comment
There was a problem hiding this comment.
- Endpoint logic looks good, addresses the issue correctly
- Code is very straightforward
- Tests could be condensed, but functionality is sufficient
This is meant to work with the Slack application, more optimizations or changes could be made with more Slack development. I've changed my mind on the original proposed revision, I don't think it's needed given our current use case. Should be ready for merge.
2 participants
Slack - Get Student Info Endpoint
This request adds an endpoint at
GET /api/slack/student_infofor the Slack Bolt application, which the student initiates to acquire the student's information. All student information is gathered from the Postgres database. Students are authenticated under the assumption that their Slack email address is either their primary email address or one of their alternate email addresses.The shape of the JSON data response resembles the following:
{ "cti_id": 1, "fname": "John", "lname": "Doe", "pname": "Johnny", "institution": "UC Berkeley", "target_year": 2025, "join_date": "2023-09-01T00:00:00", "gender": "Male", "ethnicities_agg": ["Hispanic", "Asian"], "birthday": "2004-05-15", "first_gen": true, "email": "john.doe@example.com", "primary_email": "john.doe@example.com", "alternate_emails": ["john.alt@example.com", "john.doe.alt@example.com"] }Issues Fixed
Notes
Tests
Unit testing has been written to cover the
fetch_student_infoservice function. Run these tests with:Checklist