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

Categories implementation #88

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

Conversation

mastersuv
Copy link
Contributor

@mastersuv mastersuv commented Dec 9, 2020

Issue #33
Hey @Steffo99
I implemented categories into your existing code trying to preserve your code style and design without altering too much the original bot. Feel free to check it out. It's been fun to develop. This was my first time programming in python and it was a great project to start learning.

Please note there could still be bugs. I'm the only one testing for now.

To enable categories set category_mode=true on config.toml

Santiago Valenzuela added 5 commits December 9, 2020 04:06
- Added line for "Non in vendita" which was previously hardcoded into worker.py for "Not for sale yet" products.
- Added strings for categories implementation.
config/template_config.toml Show resolved Hide resolved
config/template_config.toml Show resolved Hide resolved
database.py Outdated Show resolved Hide resolved
@Steffo99
Copy link
Owner

This seems like an huge pull request. Thanks! :D

I'll take some time to check it out tomorrow :)

@Steffo99 Steffo99 self-requested a review December 10, 2020 00:48
@Steffo99 Steffo99 self-assigned this Dec 10, 2020
@Steffo99 Steffo99 linked an issue Dec 10, 2020 that may be closed by this pull request
@Steffo99 Steffo99 added the feature A request for a new feature. label Dec 10, 2020
Copy link
Owner

@Steffo99 Steffo99 left a comment

Choose a reason for hiding this comment

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

Here's some feedback on this PR :)

I haven't tested it yet, but it seems promising at a first glance :D

@@ -0,0 +1,9 @@
{
Copy link
Owner

Choose a reason for hiding this comment

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

You might want to avoid committing .vs files :)

Add .vs/ to the .gitignore!

config/template_config.toml Show resolved Hide resolved
Comment on lines +95 to +96
# Category id
category_id = Column(Integer, ForeignKey("categories.id"))
Copy link
Owner

Choose a reason for hiding this comment

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

Since you added a column to an already existing table, this change will break compatibility with older versions, as there is no way to perform database migrations... (#74)

I'll need to find a solution to that before merging this pull request :S

Comment on lines +89 to +94
# Conversation: select a category to edit
conversation_admin_select_category = "✏️ What category do you want to edit?"

# Conversation: select a category to delete
conversation_admin_select_category_to_delete = "❌ What category do you want to delete?"

Copy link
Owner

Choose a reason for hiding this comment

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

Note to myself: I should add those lines to the Italian translation too, or it will stop working as a fallback language.

Comment on lines +502 to +503
CategoryMode = self.cfg["Mode"]["category_mode"]
FinalStep = False
Copy link
Owner

Choose a reason for hiding this comment

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

Variable names should be snake_case (all lowercase, with underscores as spaces)

Suggested change
CategoryMode = self.cfg["Mode"]["category_mode"]
FinalStep = False
category_mode = self.cfg["Mode"]["category_mode"]
final_step = False

@@ -896,12 +1014,18 @@ def __admin_menu(self):
"""Function called from the run method when the user is an administrator.
Administrative bot actions should be placed here."""
log.debug("Displaying __admin_menu")
CategoryMode = self.cfg["Mode"]["category_mode"]
Copy link
Owner

Choose a reason for hiding this comment

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

snake_case this too, please :)

@@ -915,16 +1039,22 @@ def __admin_menu(self):
reply_markup=telegram.ReplyKeyboardMarkup(keyboard, one_time_keyboard=True))
# Wait for a reply from the user
selection = self.__wait_for_specific_message([self.loc.get("menu_products"),
self.loc.get("menu_categories"),
Copy link
Owner

Choose a reason for hiding this comment

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

Let's make that even with the other lines.

Suggested change
self.loc.get("menu_categories"),
self.loc.get("menu_categories"),

# End the process
sys.exit(0)
sys.exit(0)
Copy link
Owner

Choose a reason for hiding this comment

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

Files should end with a newline.

Suggested change
sys.exit(0)
sys.exit(0)

@Steffo99 Steffo99 removed their assignment May 19, 2021
@Steffo99 Steffo99 added the waiting Nothing can be done for this issue except waiting for an external situation to change. label Sep 4, 2021
@Steffo99
Copy link
Owner

Steffo99 commented Sep 4, 2021

Blocked by #74.

@AnonymousF0x
Copy link

can you please add sub Categories ???

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A request for a new feature. waiting Nothing can be done for this issue except waiting for an external situation to change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a way to categorize products
3 participants