Skip to content

THUND-174: Default to immediate initialization of the association#233

Closed
phurley67 wants to merge 1 commit intomasterfrom
THUND-174-handle-models-that-only-include-owner
Closed

THUND-174: Default to immediate initialization of the association#233
phurley67 wants to merge 1 commit intomasterfrom
THUND-174-handle-models-that-only-include-owner

Conversation

@phurley67
Copy link
Contributor

https://invoca.atlassian.net/browse/THUND-174

For some reason oauth_provider uses the large_text_field gem; however, it does not actually define any fields. Adjust the gem to support this use case.

@phurley67 phurley67 requested a review from hislopzach June 11, 2025 17:49
@phurley67 phurley67 marked this pull request as ready for review June 11, 2025 17:49
@phurley67 phurley67 requested a review from a team as a code owner June 11, 2025 17:49
@phurley67 phurley67 requested a review from jebentier June 11, 2025 17:49
Copy link
Contributor

@jebentier jebentier left a comment

Choose a reason for hiding this comment

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

I'm not following the root cause need for this change. What changed that made this necessary to be supported? Can we just remove includes of LargeTextField::Owner in models that don't use the large_text_field? Likely that was missed in a migration off of large text field in the past.

@phurley67
Copy link
Contributor Author

I probably would have done that, but the offending code was in a different gem, and I was trying to minimize the foot print of my changes.

This also keeps the gem semantically the same as it was in 1.2.

The other gem is oauth_provider

@jebentier
Copy link
Contributor

jebentier commented Jun 12, 2025

So it's easier to update this gem, over the other gem?

But also, what changed that made this edge case in the first place? This and the oauth_provider gem haven't changed in a long time.

@phurley67
Copy link
Contributor Author

Sorry I thought I replied, but something failed. I made the 1.3 version, that changed the semantics which exposed the issue, so I wanted to "fix" that. But I think you are correct that updating the oauth gem is the correct path.

@phurley67 phurley67 closed this Jun 13, 2025
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.

2 participants