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.

Sign up for GitHub

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

Jump to bottom

Remove area and change name from InspectionArea to InspectionGroup #2003

Closed
wants to merge 1 commit into from
Closed

Remove area and change name from InspectionArea to InspectionGroup #2003

wants to merge 1 commit into from
+1,729 −3,235

Conversation

Christdej
Copy link
Contributor

Ready for review checklist:

  • A self-review has been performed
  • All commits run individually
  • Temporary changes have been removed, like console.log, TODO, etc.
  • The PR has been tested locally
  • A test have been written
    • This change doesn't need a new test
  • Relevant issues are linked
  • Remaining work is documented in issues
    • There is no remaining work from this PR that require new issues
  • The changes does not introduce dead code as unused imports, functions etc.

Sorry, something went wrong.

@Christdej Christdej added backend Backend related functionality breaking-change A breaking change which introduces changes to the public APIs frontend Frontend related functionality database-change Will require migration labels Feb 3, 2025
@Christdej Christdej self-assigned this Feb 3, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 3, 2025

🔔 Changes in database folder detected 🔔
Do these changes require adding new migrations? 🤔 In that case follow these steps.
If you are uncertain, ask a database admin on the team 😄

Eddasol
Eddasol reviewed Feb 3, 2025
View reviewed changes
backend/api/Database/Context/FlotillaDbContext.cs
.Entity<InspectionArea>()
.HasOne(d => d.Plant)
.Entity<InspectionGroup>()
.HasOne(d => d.Installation)
Copy link
Contributor

Choose a reason for hiding this comment

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

d (for deck) should probably be changed as well. Have notices a few of those in frontend as well

andchiind
andchiind reviewed Feb 3, 2025
View reviewed changes
Copy link
Contributor

@andchiind andchiind left a comment

Choose a reason for hiding this comment

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

It could be good to split up the commits so that files that are edited and renamed don't appear to be deleted. I understand that this is just a draft though.

@Christdej Christdej force-pushed the area branch 3 times, most recently from 7c22cf6 to 7ae9fc7 Compare February 4, 2025 08:50
Eddasol
Eddasol reviewed Feb 5, 2025
View reviewed changes
backend/api/Services/InspectionGroupService.cs
if (existingInspectionGroup != null)
{
throw new InspectionGroupExistsException(
$"Inspection are with name {newInspectionGroupQuery.Name} already exists"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$"Inspection are with name {newInspectionGroupQuery.Name} already exists"
$"Inspection group with name {newInspectionGroupQuery.Name} already exists"

@Christdej Christdej closed this Feb 10, 2025
@Christdej Christdej deleted the area branch February 10, 2025 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andchiind andchiind

@Eddasol Eddasol

Assignees

@Christdej Christdej

Labels
backend Backend related functionality breaking-change A breaking change which introduces changes to the public APIs database-change Will require migration frontend Frontend related functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Christdej @andchiind @Eddasol