Skip to content
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

PIMS-401: Create ORM Entity Models #1989

Merged
merged 84 commits into from
Jan 12, 2024
Merged

PIMS-401: Create ORM Entity Models #1989

merged 84 commits into from
Jan 12, 2024

Conversation

Sharala-Perumal
Copy link
Collaborator

@Sharala-Perumal Sharala-Perumal commented Dec 19, 2023

PIMS-401: Create ORM Entity Models

🎯 Summary

PIMS-401

DB Diagram

  • AccessRequests
  • AdministrativeAreas
  • Addresses
  • Agencies
  • BuildingConstructionTypes
  • BuildingEvaluations
  • BuildingFiscals
  • BuildingOccupantTypes
  • BuildingPredominantUses
  • Buildings
  • Claims
  • NotificationQueue
  • NotificationTemplates
  • Parcels
  • ParcelBuildings
  • ParcelEvaluations
  • ParcelFiscals
  • ParcelParcels (renamed to Subdivisons)
  • ProjectAgencyResponses
  • ProjectNotes
  • ProjectNumbers
  • ProjectProperties
  • ProjectReports
  • ProjectRisks
  • Projects
  • ProjectSnapshots
  • ProjectStatus
  • ProjectStatusHistory
  • ProjectStatusNotifications
  • ProjectStatusTransitions
  • ProjectTasks
  • ProjectTypes
  • PropertyClassifications
  • PropertyTypes
  • Provinces
  • RegionalDistricts
  • ReportTypes
  • RoleClaims
  • Roles
  • Tasks
  • TierLevels
  • UserAgencies
  • UserRoles
  • Users
  • WorkflowProjectStatus
  • Workflows

🔰 Checklist

  • I have read and agree with the following checklist and am following the guidelines in our Code of Conduct document.
  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation where required.
  • I have tested my changes to the best of my ability.
  • My changes generate no new warnings.

@Sharala-Perumal Sharala-Perumal self-assigned this Dec 19, 2023
Copy link

codeclimate bot commented Dec 19, 2023

Code Climate has analyzed commit adbbfd1 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 93.1%.

View more on Code Climate.

@dbarkowsky
Copy link
Collaborator

Running into issues with the OneToMany additions. Decided to get this merged so other tickets can be started, and we can address the OneToMany options later if we feel the need to include them.

@dbarkowsky dbarkowsky marked this pull request as ready for review January 10, 2024 23:56
@dbarkowsky dbarkowsky changed the title PIMS-401: Create ORM Entity Models **DRAFT ONLY - DO NOT MERGE** PIMS-401: Create ORM Entity Models Jan 10, 2024
@GrahamS-Quartech
Copy link
Contributor

I'll just note that I think it is more common to use snake case for table / column names in Postgres, and at least in the natural resource ministries they have guidelines recommending as much. Not sure if similar DB guidelines exist for this ministry. Ultimately though these conventions are arbitrary and consistency is the more important thing at the end of the day.

@dbarkowsky
Copy link
Collaborator

I'll just note that I think it is more common to use snake case for table / column names in Postgres, and at least in the natural resource ministries they have guidelines recommending as much. Not sure if similar DB guidelines exist for this ministry. Ultimately though these conventions are arbitrary and consistency is the more important thing at the end of the day.

This is a good point. The table names appear to get converted to snake case in the database once built, but the column names don't. DPIA had specified the column names to address this. https://github.com/bcgov/cirmo-dpia/blob/main/src/backend/src/modules/ppq/entities/ppq.entity.ts

Current PIMS database uses PascalCase for table names and columns.

@LawrenceLau2020 @Sharala-Perumal Do we feel the need to address this?

@LawrenceLau2020
Copy link
Collaborator

I'll just note that I think it is more common to use snake case for table / column names in Postgres, and at least in the natural resource ministries they have guidelines recommending as much. Not sure if similar DB guidelines exist for this ministry. Ultimately though these conventions are arbitrary and consistency is the more important thing at the end of the day.

This is a good point. The table names appear to get converted to snake case in the database once built, but the column names don't. DPIA had specified the column names to address this. https://github.com/bcgov/cirmo-dpia/blob/main/src/backend/src/modules/ppq/entities/ppq.entity.ts

Current PIMS database uses PascalCase for table names and columns.

@LawrenceLau2020 @Sharala-Perumal Do we feel the need to address this?

I'm good either way.

@Sharala-Perumal
Copy link
Collaborator Author

I'll just note that I think it is more common to use snake case for table / column names in Postgres, and at least in the natural resource ministries they have guidelines recommending as much. Not sure if similar DB guidelines exist for this ministry. Ultimately though these conventions are arbitrary and consistency is the more important thing at the end of the day.

This is a good point. The table names appear to get converted to snake case in the database once built, but the column names don't. DPIA had specified the column names to address this. https://github.com/bcgov/cirmo-dpia/blob/main/src/backend/src/modules/ppq/entities/ppq.entity.ts
Current PIMS database uses PascalCase for table names and columns.
@LawrenceLau2020 @Sharala-Perumal Do we feel the need to address this?

I'm good either way.

As long as the case is consistent across tables and columns , I am okay either ways unless there is an obvious advantage of using one over the other.

Copy link
Collaborator

@dbarkowsky dbarkowsky left a comment

Choose a reason for hiding this comment

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

No objections as of yet from team. Merging this as a first draft for now so we have the entities to work with.

@dbarkowsky dbarkowsky merged commit bde0e11 into main Jan 12, 2024
3 checks passed
@dbarkowsky dbarkowsky deleted the PIMS-401 branch January 12, 2024 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants