WIP: Create Table `permission_type`

[joey-ma]

Overview

Fixes issue #24.

This is a work in progress, just wanted to push it first so reviewers can leave comments early on if desired.

Details

Action Items

Following the guide, I think these are bullet points are helpful to follow. Will refer back to original issue for details.

• [x] Add the model in app/core/models.py
  • [x] Run migrations to generate database migration files
  • [x] Write some tests
  • [x] Add fixtures (in app/core/tests/conftest.py)
  • [x] Add tests (in app/core/tests/test_models.py)
  • [x] Passing tests (with good coverage)
  • [x] Run pre-commit checks ./scripts/precommit-check.sh
• [x] Register the model
• [x] Add serializer in app/core/api/serializers.py
• [x] Add view set in app/core/api/views.py
• [x] Register API endpoints to the router in app/core/api/urls.py
  • [ ] Write some tests
  • [ ] Add a test (in app/core/tests/test_api.py)
  • [ ] Passing tests (with good coverage)
  • [ ] Run pre-commit checks ./scripts/precommit-check.sh
• [ ] Review other requirements from original issue #24

[joey-ma]

@fyliu (1) For adding a viewset and CRUD API endpoint descriptions, should I follow the format of the Extended example or is the basic version good?

(2) What would be the query needed here?

[joey-ma]

Question:

• Would someone be able to point me to some resource on how to write a test for registering API endpoints to the router?

Comment:

• Is issue better done after creating the permission table since that references this table?

[fyliu]

@fyliu (1) For adding a viewset and CRUD API endpoint descriptions, should I follow the format of the Extended example or is the basic version good?

The basic is good. I included that to show what can be done if a stakeholder were to require those functionalities.

(2) What would be the query needed here?

Queries? We don't need to write queries for standard actions that django provides. We could define custom queries if we need to optimize things later on.

[fyliu]

Question:

• Would someone be able to point me to some resource on how to write a test for registering API endpoints to the router?

Adding a test in test_api.py could work for the CRUD actions. Or do you want to make sure the endpoints are showing up at the correct url? Like permission-types/ instead of permission-typeseses/? Maybe something like assert(reverse("permission-type-list") == "[some base url string]/permission-types/"). Currently, we're just relying on the code reviews to catch these.

Comment:

• Is issue better done after creating the permission table since that references this table?

No. It's better that this one exists before doing the other one so that one won't have to be worked on a second time to add the relationship reference.

[fyliu]

Question:

• Would someone be able to point me to some resource on how to write a test for registering API endpoints to the router?

Adding a test in test_api.py could work for the CRUD actions. Or do you want to make sure the endpoints are showing up at the correct url? Like permission-types/ instead of permission-typeseses/? Maybe something like assert(reverse("permission-type-list") == "[some base url string]/permission-types/"). Currently, we're just relying on the code reviews to catch these.

I think I misunderstood your question. You can follow this guide to see the new endpoints at step 8.

[joey-ma]

@fyliu What is left to do in this draft PR is the test portion of API routes. This is translated from the "write API unit tests" check box / requirement from issue #24. This is why I'm bringing up questions around how to write tests for the API endpoints. I was able to follow the Cognito authentication workflow (pre deployment) page, but am not confident that I got things working the way it's intended. I'll need more time to investigate/explore around this.

However, given my work and family situation, I don't have much time to work on this anymore. If there's someone who will be able to finish what I've started, I think that might be more productive.

If not, I don't mind continue working on this, but might only be able to find a few hours (if I'm lucky) in the next 2 weeks to finish the rest of the bullet points.

[fyliu]

@alexlaw528 continued working on this and created PR #201. Closing this PR and continuing there.