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 add_favorites command #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add add_favorites command #8

wants to merge 2 commits into from

Conversation

jonchan51
Copy link
Contributor

@jonchan51 jonchan51 commented Feb 1, 2020

TODO:

  • Change to remove callback to an edit

Copy link
Contributor

@moziliar moziliar left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

update.message.reply_text(add_favorite_already_exists_msg())
return

update.message.reply_text(added_favorites_msg(favorites))
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we reduce verbosity with one-liner
update.message.reply_text(added_favorites_msg(favorites) if favorites is None else add_favorite_already_exists_msg())

update.message.reply_text(no_favorites_msg())
return

update.message.reply_text('Select your favorite food to remove:', reply_markup=favorites_kb(favorites))
Copy link
Contributor

Choose a reason for hiding this comment

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

Abstract away the reply text to const.py or message.py?

message_id=update.callback_query.message.message_id, reply_markup=favorites_kb(favorites))
else:
context.bot.edit_message_text(text=favorites_msg(favorites), chat_id=update.effective_chat.id,
message_id=update.callback_query.message.message_id, reply_markup=favorites_kb(favorites))
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above the one-liner?

favorites = data[FAVORITES]

cursor.close()
return favorites # returns favorites in user_pref
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we practice DRY by taking in the type of user_pref as a function parameter and extract data accordingly?
These few lines are almost exactly the same as line 65-79

cursor = conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor)
cursor.execute(settings_update(FAVORITES), (favorites, chat_id))
conn.commit()
cursor.close()
Copy link
Contributor

Choose a reason for hiding this comment

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

Likewise, can we DRY the updating process?

NOTIFICATION = 'notification'
HOME = 'home'
HIDE_CUISINE = 'hidden'
ADD_FAVORITE = 'add_favorite'
REMOVE_FAVORITE = 'remove_favorite'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be Singapore readable 'favourite' for the users or left Americanized?

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