-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update URL to url in field_type enum #1032
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CREATE TYPE updated_field_type AS ENUM ('string', 'number', 'phone_number', 'email', 'url', 'boolean'); | ||
ALTER TABLE base_fields | ||
ALTER COLUMN data_type TYPE updated_field_type USING | ||
CASE | ||
WHEN data_type = 'URL' THEN 'url'::updated_field_type | ||
ELSE data_type::text::updated_field_type | ||
END; | ||
DROP TYPE field_type CASCADE; | ||
ALTER TYPE updated_field_type RENAME to field_type; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ export enum BaseFieldDataType { | |
NUMBER = 'number', | ||
PHONE_NUMBER = 'phone_number', | ||
EMAIL = 'email', | ||
URL = 'URL', | ||
URL = 'url', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎉 |
||
BOOLEAN = 'boolean', | ||
} | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. I thought for a minute there may be another more brief way of doing it but this is probably the straightest-forward approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, and I have no qualms. I was curious though so asked GPT how it would do it:
It's very similar to your approach, but by adding the
url
value to the original type / updating the fields to useurl
, and then swapping the enum to one without the (now unusedURL
) you could avoid the complexity of theCASE
statement.I defer to you @hminsky2002 on which you prefer!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slifty I think that avoiding a case statement is preferable!
I shall edit this commit with gusto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following up here since Harry and I went offline to do some rapid back and forth -- Harry did implement this change but psql doesn't like using enums that aren't committed in a transaction (idk why, but whatever.)
Having two migrations felt nastier than a case statement, so he landed back where he started with the case statement!
That was fun :D