Skip to content

Feature/name change#650

Open
drakeredwind01 wants to merge 12 commits intomainfrom
feature/name_change
Open

Feature/name change#650
drakeredwind01 wants to merge 12 commits intomainfrom
feature/name_change

Conversation

@drakeredwind01
Copy link
Copy Markdown
Member

Fixes #607

What changes did you make?

Current name in code Updated Type (may already be this type) FK Table
github_org_id ForeignKey ProjectUrl
github_primary_repo_id ForeignKey ProjectUrl
google_drive_id ForeignKey ProjectUrl
completed_on DateField N/A
project_leads ManyToManyField User

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

for issue Update Table: project #607

@fyliu fyliu moved this to PR Needs review (automated column, do not place items here manually) in P: PD: Project Board Mar 18, 2026
@fyliu fyliu moved this from PR Needs review (automated column, do not place items here manually) to 👀PR being reviewed in P: PD: Project Board Mar 18, 2026
Copy link
Copy Markdown
Member

@fyliu fyliu left a comment

Choose a reason for hiding this comment

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

Great for the most part. Just a few little details need fixing.

  • Building the image generates a new migration file that should've been added.

  • the 3 fields that turned into FKs need tests.

  • The project_leads is missing tests. But as a many to many field, I think it's better to split it into a separate issue to work on. I think I'll need to look at a few ways of doing this and choose one that makes sense for us.

@github-project-automation github-project-automation bot moved this from 👀PR being reviewed to PR changes requested in P: PD: Project Board Mar 19, 2026
drakeredwind01 and others added 4 commits March 19, 2026 17:23
removing completed_on

Co-authored-by: Fang Yi Liu <fyliu@users.noreply.github.com>
commit suggestion

Co-authored-by: Fang Yi Liu <fyliu@users.noreply.github.com>
added 
`from django.conf import settings`
drakeredwind01 and others added 3 commits March 19, 2026 17:29
altered imports
```
import uuid
+ import textwrap
+ from django.conf import settings
```
altering models.py
```
- "User",  # Replace "User" with your specific User model name if different
+ settings.AUTH_USER_MODEL
```

Co-authored-by: Fang Yi Liu <fyliu@users.noreply.github.com>
@drakeredwind01 drakeredwind01 requested a review from fyliu March 20, 2026 01:15
@fyliu
Copy link
Copy Markdown
Member

fyliu commented Mar 20, 2026

@drakeredwind01 I looked through the tables and fields spreadsheet and the ERD for the database. It looks like these fields being foreign keys pointing to ProjectUrl objects is correct. The names should be without the "_id" in them I think.

There's something unusual about these fields, and I'll try to explain with one of them.

For the google_drive_id, we have a ProjectUrl object in the database that says this project has a Google Drive link to the URL. That has the complete information without the Project object having a google_drive_id. But we're keeping this link to a specific ProjectUrl object for quick access.

So we can do something like this on just the Project object without having to look it up in the ProjectUrl table:

  • people_depot = Project.objects.get("People Depot")
  • pd_google_drive = people_depot.google_drive_id
  • pd_github_primary = people_depot.github_primary_repo

Copy link
Copy Markdown
Member

@fyliu fyliu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Sorry to do this, but can you remove the _id from the foreign key fields?

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.

Update Table: project

3 participants