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

Quote the readable.id in Unread::Reader::Scopes #108

Merged
merged 2 commits into from
Aug 25, 2019

Conversation

fotos
Copy link
Contributor

@fotos fotos commented Aug 3, 2019

This PR fixes the generated SQL when the readable.id is a UUID.

For example when running:

> user.have_read?(book)

the generated SQL is:

  Punter Exists (0.7ms)  SELECT  1 AS one FROM "people" LEFT JOIN "read_marks"
                ON "read_marks".readable_type  = 'Book'
               AND ("read_marks".readable_id   = a4691c66-344f-4ab1-8dd4-16614f6cbb52 OR "read_marks".readable_id IS NULL)
               AND "read_marks".reader_id      = "user"."id"
               AND "read_marks".reader_type    = 'User'
               AND "read_marks".timestamp     >= '2019-08-03 10:43:02.138041' WHERE "people"."type" IN ('User') AND ("read_marks".id IS NULL) AND "people"."id" = $1 LIMIT $2  [["id", "cf7144b0-2814-48d3-ba17-b46a00b46246"], ["LIMIT", 1]]

and it fails with the following error:

ActiveRecord::StatementInvalid: PG::SyntaxError: ERROR:  syntax error at or near "f"
LINE 3: ...    AND ("read_marks".readable_id   = a4691c66-344f-4ab1-8dd...
                                                             ^

Quoting the readable.id fixes the problem. Related to #60.

@coveralls
Copy link

coveralls commented Aug 3, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 8fde098 on fotos:fix-readable-id-for-uuid into 161a609 on ledermann:master.

@fotos
Copy link
Contributor Author

fotos commented Aug 23, 2019

@ledermann is there anything else I need to change in this PR (e.g. changelog, tests)?

We'd love to have the fix merged / released upstream instead of forking the gem for this (small) bug.

Would appreciate some feedback. 🙏

@ledermann
Copy link
Owner

Sorry for the late answer. Before merging your changes I want to have the tests passing. I would like to ask you to rebase the PR against master, because I just made some Travis related fixes.

@fotos
Copy link
Contributor Author

fotos commented Aug 24, 2019

Great thanks, the tests are passing now! Let me know if there is anything else.

@ledermann ledermann merged commit b3e257b into ledermann:master Aug 25, 2019
@ledermann
Copy link
Owner

Merged, thanks for contribution!

@fotos fotos deleted the fix-readable-id-for-uuid branch August 25, 2019 16:34
HwakyoungLee pushed a commit to grepp/unread that referenced this pull request May 12, 2023
* Quote the readable.id in Unread::Reader::Scopes

Fixes an SQL syntax error when the readable.id is a UUID.

* Add parantheses around quote_bound_value argument in Unread::Reader::Scopes
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

3 participants