-
Notifications
You must be signed in to change notification settings - Fork 80
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
Fixing base and user objects #1323
Fixing base and user objects #1323
Conversation
sql = """SELECT EXISTS( | ||
SELECT * FROM qiita.{0} | ||
WHERE {0}_id=%s)""".format(self._table) | ||
TRN.add(sql, [id_]) |
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.
I am under the impression that these had to be tuples and that lists were interpreted in a very different way.
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.
Nope, here we just need to pass an iterable so psycopg doesn't fail. It is weird but psycopg always requires an iterable as sql_args, even if there is a single element. Also, the transaction object forces this to be lists so it can do the placeholder replacements.
This looks good overall, very simple changes and I like that you are reformatting the SQL to be structured in a consistent way, perhaps you should take some notes and add a section in the CONTRIBUTING.md file. Also minor note, would perhaps be good to have all SQL keywords be all-caps. Other reviewers, these are very simple changes so it's quick to go through. |
@ElDeveloper comments addressed! |
Re: CONTRIBUTING.md, I've added another item to #1262 |
info = conn_handler.execute_fetchone(sql, (email, )) | ||
|
||
# verify user email verification | ||
# MAGIC NUMBER 5 = unverified email |
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.
Not your fault, but could you change this to use the new convert_to_id() function?
👍 |
1 similar comment
👍 |
Fixing base and user objects
This fixes the base and user object to use the transaction.
Since this is the first time the transaction object is used in the code base, I will appreciate if people can take a quick look to see that everything works as expected.
Thanks!