-
-
Notifications
You must be signed in to change notification settings - Fork 33
Add Accomplishment model and API endpoints for Feature #48 #530
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only thing still missing that I can tell is a test for the new accomplishment model.
Attaching a link to the tutorial I normally follow as a checklist of things to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-requesting. hit the request review accidentally
ab51173 to
1637d67
Compare
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Major comments that need to be resolved before anything else.
|
|
||
| .vscode | ||
| TODO.md | ||
| TODO.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is TODO.md added? I don't see it in the repo
| @@ -1 +1,2 @@ | |||
| 0042_urlstatustype_projecturl_url_status_type | |||
| 0039_accomplishment | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| uuid = models.UUIDField( | ||
| primary_key=True, default=uuid.uuid4, editable=False, unique=True | ||
| ) | ||
| created_at = models.DateTimeField("Created at", auto_now_add=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did we remove these fields? these are used by many other models. This looks related to the snapshot that I took in the max_migration.txt comment.
Fixes #48
What changes did you make?
Why did you make the changes (we will use this info to test)?
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
Visuals before changes are applied
N/A (No visual changes, backend implementation only)
Visuals after changes are applied
N/A (No visual changes, backend implementation only)
Files Modified/Added:
app/core/api/serializers.pyapp/core/api/views.pyapp/core/api/urls.pyapp/core/admin.pyapp/core/tests/test_api.pyapp/core/migrations/0039_accomplishment.pyReference: