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

🧹 Utility: Use UtilityHookup#location in forms #1179

Merged
merged 1 commit into from
Mar 4, 2023

Conversation

zspencer
Copy link
Member

@zspencer zspencer commented Mar 3, 2023

This is mostly for consistency and to start to close out a piece of work I started a while back but never finished.

- Extracted from htps://github.com//pull/1083

This is mostly for consistency and to start to close out a piece of work
I started a while back but never finished.
@zspencer zspencer added the 🧹 refactor Includes non-behavioral changes label Mar 3, 2023
@zspencer zspencer requested review from KellyAH and a team March 3, 2023 23:44
@@ -4,6 +4,7 @@
class UtilityHookup < ApplicationRecord
# @return [Space]
belongs_to :space
self.location_parent = :space
Copy link
Contributor

@KellyAH KellyAH Mar 3, 2023

Choose a reason for hiding this comment

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

I see thatlocation_parent was created for rails routing... but why do we need it? What is it logically doing?

def location_parent

Copy link
Member Author

@zspencer zspencer Mar 4, 2023

Choose a reason for hiding this comment

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

It provides a url hierarchy when leveraging polymorphic routes. For example: /spaces/:space_id/utility_hookups vs /utility_hookups/

Copy link
Contributor

Choose a reason for hiding this comment

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

ok within_location.rb seems like a custom implementation and not something that come with rails.

I'll learn more about polymorphic routes on my own.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, rails does not include it by default because most Rails applications are flat; and it's often an anti-pattern to have too deeply nested of resources!

However, we need to keep models within the Room or Space; so we almost always have at least those two levels of nesting; plus any additional domain-specific nesting that Furniture may want.

@zspencer zspencer changed the title 🧹 Utility: Use #location in forms 🧹 Utility: Use UtilityHookup#location in forms Mar 4, 2023
@zspencer zspencer requested a review from KellyAH March 4, 2023 00:16
@zspencer zspencer merged commit 600f633 into main Mar 4, 2023
@zspencer zspencer deleted the utilities/support-byo-domains-better branch March 4, 2023 00:46
@zspencer zspencer added this to the 1.0 - Andromeda milestone May 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 refactor Includes non-behavioral changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants