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

Create a generic point component type #1436

Merged
merged 6 commits into from
Oct 10, 2024
Merged

Conversation

mddilley
Copy link
Collaborator

@mddilley mddilley commented Sep 30, 2024

Associated issues

Closes cityofaustin/atd-data-tech#19132

This PR:

  • renames the existing line-type generic extent component to Project Extent - Generic (linear)
  • adds a new point-type generic extent component called Project Extent - Generic

Testing

URL to test:

Local

Steps to test:

  1. Start your local stack
  2. Create a new project and go to the Map tab
  3. Add a new component and notice that there are now two generic project extent types: Project Extent - Generic and Project Extent - Generic
    • Project Extent - Generic has a point type icon next to it in the dropdown
    • Project Extent - Generic (linear) has a line type icon next to it in the dropdown
  4. Check the component work types in the form to make sure they both have: New, Modification, and Maintenance / Repair which can be saved and edited
  5. Run the down migration which *deletes the new generic component and reclassifies any components created as generic points as intersection improvement point components soft deletes the new component type and restores the old naming of the existing generic component type. The up migration sets is_deleted = false on the generic point type so we can make use of the existing is_deleted column in moped_components to track what we've added/removed. Does this make sense? I went 😵‍💫 when thinking about the different cases.

Ship list

@mddilley mddilley added the WIP Work in progress label Sep 30, 2024
Copy link
Member

@chiaberry chiaberry left a comment

Choose a reason for hiding this comment

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

The work types do not include replacement, but include maintenance / repair. I think the test instructions are incorrect, because this does match what is in production for generic type now.
Screenshot 2024-10-04 at 9 21 50 AM

As for restoring the point type component if it was soft deleted, this would only happen if the down migration was run at some point and then we needed to run the up migration again, right?

I'll mark this approved after its confirmed that we didnt need to update the work types for the generic project extent line.

Copy link
Member

@johnclary johnclary left a comment

Choose a reason for hiding this comment

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

i approve but also +1 to Chia's comments about the work types 🙏 🚢

@@ -0,0 +1,5 @@
UPDATE moped_components SET is_deleted = true WHERE component_name = 'Project Extent - Generic' AND line_representation = false;
Copy link
Member

Choose a reason for hiding this comment

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

in the spirit of having the down migrations restore the DB to the previous state, i think the right move here would be to actually hard delete the component that the up migration created. obviously, that's going to fail if there are any moped_proj_components that reference the component, so here are a few ideas:

  • just acknowledge that the down migration will not work without manual intervention by the dev to deal with the extant moped_proj_components that reference it
  • write the down migration to also migrate the moped_proj_components to some other component type (like intersection improvement generic)
  • write the down migration to hard delete moped_proj_components which use this component type, yolo style

in any case, i don't think this is a big deal so i'm going to approve your PR! 🤷

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cool - thanks for saying this, and i agree that the hard delete is the better call to get back to pre-up migration state. I'm going to make that change since the soft-delete/un-soft-delete felt weird in the first place! probably the reason my spidey sense was going off 🕷️

@mddilley
Copy link
Collaborator Author

mddilley commented Oct 7, 2024

@chiaberry Thank you for catching that detail in the test instructions. 🙏 I just corrected them to include Maintenance / Repair and not Replacement. I think that I copypasta'd that from a different component type.

Copy link
Member

@chiaberry chiaberry left a comment

Choose a reason for hiding this comment

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

🚢 generic 👉🏼

Copy link
Contributor

@tillyw tillyw left a comment

Choose a reason for hiding this comment

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

tested locally and the new generic component and naming conventions look great! can add and edit work types as expected. ran the down migration and the records i created reverted without issue. thanks for making this update! ship ship ship

@mddilley
Copy link
Collaborator Author

thanks y'all - merging!

@mddilley mddilley merged commit c721743 into main Oct 10, 2024
5 checks passed
@mddilley mddilley deleted the md-19132-generic-point-component branch October 10, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a generic point component type
4 participants