Skip to content

Conversation

@drakeredwind01
Copy link
Member

@drakeredwind01 drakeredwind01 commented Sep 3, 2025

Fixes #48

What changes did you make?

  • Added Accomplishment model with proper fields and relationships
  • Added API endpoints with serializers, views, and URL routing
  • Added Django admin registration for the new model
  • Implemented comprehensive test coverage utilizing test fixtures
  • Made database migration

Why did you make the changes (we will use this info to test)?

  • To implement the Accomplishment model as specified in issue Create Table: Accomplishment #48
  • To provide REST API endpoints for managing Accomplishment records
  • To ensure proper integration with the existing codebase
  • To maintain code quality with comprehensive test coverage

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:

File Changes
app/core/models.py Added Accomplishment model
app/core/api/serializers.py Added AccomplishmentSerializer
app/core/api/views.py Added AccomplishmentViewSet
app/core/api/urls.py Added accomplishments URL routing
app/core/admin.py Added admin registration
app/core/tests/test_api.py Added test function
app/core/tests/conftest.py Added Accomplishment fixture
app/core/migrations/0039_accomplishment.py Created migration

Reference:

@drakeredwind01 drakeredwind01 mentioned this pull request Sep 3, 2025
18 tasks
@fyliu fyliu moved this to PR Needs review (automated column, do not place items here manually) in P: PD: Project Board Sep 3, 2025
@fyliu fyliu requested a review from dmartin4820 September 5, 2025 01:12
Copy link
Member

@dmartin4820 dmartin4820 left a 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.

@github-project-automation github-project-automation bot moved this from PR Needs review (automated column, do not place items here manually) to PR changes requested in P: PD: Project Board Sep 8, 2025
@dmartin4820 dmartin4820 self-requested a review September 16, 2025 01:19
Copy link
Member

@dmartin4820 dmartin4820 left a 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

@drakeredwind01 drakeredwind01 force-pushed the Feature/48-drakeredwind01-accomplishment branch from ab51173 to 1637d67 Compare October 24, 2025 00:50
Copy link
Member

@dmartin4820 dmartin4820 left a 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
Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

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

Does this run without error for you? I tried running your code but I get these errors shown below:

drake_issue_48.

uuid = models.UUIDField(
primary_key=True, default=uuid.uuid4, editable=False, unique=True
)
created_at = models.DateTimeField("Created at", auto_now_add=True)
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: PR changes requested

Development

Successfully merging this pull request may close these issues.

Create Table: Accomplishment

3 participants