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

Add support for citext arrays #43

Merged
merged 5 commits into from
Apr 8, 2023
Merged

Conversation

eneagoe
Copy link
Contributor

@eneagoe eneagoe commented Apr 5, 2023

Currently if the tags column is of citext type, the generated SQL is invalid. Adding this extra configuration fixes the issue.

@skatkov
Copy link
Collaborator

skatkov commented Apr 6, 2023

Can you add some test as well, please? And a changelog entry.

@eneagoe
Copy link
Contributor Author

eneagoe commented Apr 6, 2023

Can you add some test as well, please? And a changelog entry.

Will do!

@eneagoe
Copy link
Contributor Author

eneagoe commented Apr 6, 2023

The new specs pass locally, but i'm afraid in order to enable the citext extension the PG user needs to be a super-user, so the specs fail on the CI server.

@skatkov
Copy link
Collaborator

skatkov commented Apr 6, 2023

@eneagoe I assume, CI could be fixed if you run application as superuser.

I ran through tests.yml configuration file:
https://github.com/tmiyamon/acts-as-taggable-array-on/blob/master/.github/workflows/tests.yml

It might be two things:

  • Password for PG user is empty, but should be "postgresql".
  • "Running a test" step might need an environment variable POSTGRES_USER.

But it's just my wild guess.

@eneagoe
Copy link
Contributor Author

eneagoe commented Apr 6, 2023

It looks a bit uglier now, but configuring the citext extension upfront for the test database should pass all the tests now - they do in my fork.

@skatkov
Copy link
Collaborator

skatkov commented Apr 6, 2023

CI is working.

But what would happen locally now, if I don't have citext enabled?

@eneagoe
Copy link
Contributor Author

eneagoe commented Apr 7, 2023

Good point - I've added a condition to enable the citext extension if it's not already enabled on the test database.

@skatkov
Copy link
Collaborator

skatkov commented Apr 8, 2023

@eneagoe thanks for contribution -- seems to be working locally for me too.

I'll merge this and release a new gem.

@skatkov skatkov merged commit 4b33b2e into tmiyamon:master Apr 8, 2023
4 checks passed
@eneagoe
Copy link
Contributor Author

eneagoe commented Apr 8, 2023

@skatkov Thank you for your time and help!

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.

None yet

2 participants