-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
Create table project_program_area_xref #376
Changes from all commits
e080425
001df1b
65fc748
9465c51
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
# Generated by Django 4.2.11 on 2024-08-29 19:28 | ||
|
||
from django.db import migrations, models | ||
import django.db.models.deletion | ||
import uuid | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
("core", "0024_checktype"), | ||
] | ||
|
||
operations = [ | ||
migrations.CreateModel( | ||
name="ProjectProgramAreaXref", | ||
fields=[ | ||
( | ||
"uuid", | ||
models.UUIDField( | ||
default=uuid.uuid4, | ||
editable=False, | ||
primary_key=True, | ||
serialize=False, | ||
unique=True, | ||
), | ||
), | ||
( | ||
"created_at", | ||
models.DateTimeField(auto_now_add=True, verbose_name="Created at"), | ||
), | ||
( | ||
"updated_at", | ||
models.DateTimeField(auto_now=True, verbose_name="Updated at"), | ||
), | ||
( | ||
"created_date", | ||
models.DateTimeField( | ||
blank=True, null=True, verbose_name="Created at" | ||
), | ||
), | ||
( | ||
"program_area_id", | ||
models.ForeignKey( | ||
on_delete=django.db.models.deletion.CASCADE, | ||
to="core.programarea", | ||
), | ||
), | ||
( | ||
"project_id", | ||
models.ForeignKey( | ||
on_delete=django.db.models.deletion.CASCADE, to="core.project" | ||
), | ||
), | ||
], | ||
options={ | ||
"abstract": False, | ||
}, | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
0024_checktype | ||
0025_projectprogramareaxref |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -383,3 +383,12 @@ class CheckType(AbstractBaseModel): | |
|
||
def __str__(self): | ||
return f"{self.name}" | ||
|
||
|
||
class ProjectProgramAreaXref(AbstractBaseModel): | ||
project_id = models.ForeignKey(Project, on_delete=models.CASCADE) | ||
program_area_id = models.ForeignKey(ProgramArea, on_delete=models.CASCADE) | ||
created_date = models.DateTimeField("Created at", null=True, blank=True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The inherited AbstractBaseModel class already creates a timestamp for creation; see here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did they use the DateTimeField in the Add the Model section of this guide: https://hackforla.github.io/peopledepot/how-to/add-model-and-api-endpoints/, and in the Project model as "completed_at", in the Event model as "start_time", and in the Affiliation model as "ended_at"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @del9ra there's some translation required between the DB notation in the ERD and spreadsheet and the Django data fields. We use an abstract base class (ABC) so that all the models will have create and modify timestamps. In the database design, many but not all models have the timestamps. We decided to make that a standard thing at one point. So you should ignore anything from the database table that does the same as create and update timestamps. At the time, I was trying to decide what to call these standard timestamps, and I found some page which discussed "on" vs "at", where "on" is usually a date and "at" is usually a time. So I added "_at" to the end of the names. But eventually I found a similar There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm... looks like this xref table doesn't have any extra data fields other than the relations (foreign keys). It means that we don't really need to create a Django model for this table at all. I'll have to go though the other issues and pick out similar issues before someone works on them. Like the issue where I noted this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @del9ra I'm sorry, but I need to change this issue from "create xref table" to "create many to many relation between Project and ProgramArea models". We don't have any existing code in the repository to serve as an example, but I can find some other examples if you want to work on it. This guide looks good for the Django model. These may be helpful also: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@del9ra Did you get your question answered? I'm not sure why those were included either. This should be added to the meeting agenda if we can't resolve this. I'm not sure of how the decisions were made for completed, start, ended and so on. Looking at the ERD also shows that completed_at has a type of "date" versus "DateTimeField". It looks like the ERD also needs to be updated for at least Project, if not all the other tables. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sounds good to me. I’ll work on the many-to-many relation. Thanks for letting me know! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@fyliu I noticed that we have the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @del9ra Thank you for bringing this up! I'm thinking we should bring up the inconsistent naming to @ExperimentsInHonesty and @Neecolaa and at least make an issue (ER maybe) to fix it on some tables. Maybe make them all We should bring this to the meeting and decide on whether or not to make an ABC model or something else to help force this consistency. Here's a list of stakeholders and what they might care about most:
|
||
|
||
def __str__(self): | ||
return f"Project Id: {self.project_id}, Program area Id: {self.program_area_id}" |
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.
To fix the issue we were encountering when running ./scripts/buildrun.sh, removing this line was just a local quick fix. Instead it may be better to specify graphviz=2.42.* so that the newest patch is installed instead (i.e. graphviz_2.42.2-5+deb11u1 instead of graphviz_2.42.2-5).