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

Feat: Optional Customizable Attribute Filters #385

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Madcheese101
Copy link

@Madcheese101 Madcheese101 commented Sep 16, 2023

First Commit : Feat: Optional Attribute Filters

  • Now has options to add two attribute filters (2 select fields)
  • Changed Item Group filter location to the top bar for better UX
settings pos

Second Commit: Fixed: filtering and attribute field not populating

  • New attribute filtering fields from the previous commit now use the item_attributes field instead of columns from SQL statement while not using the use_limit_search field.
  • Fixed an issue where the condition for fetching variant item attributes was incorrect. (see note below)
  • item_attributes array for variants items is now an array of strings instead of an array of dictionaries (see note below)

Note: Please make sure that the fixes in code starting from line 310 in file posawesome/posawesome/api/posapp.py do not break any other code logic

- Now has options to add two attribute filters (2 select fields)
- Changed Item Group filter location to the top bar for better UX
- new attribute filtering fields from previous commit  now use the item_attributes field instead on columns from SQL statement while not using the use_limit_search field.

- fixed an issue where the condition for fetching variant item attributes was incorrect.
@Madcheese101
Copy link
Author

Madcheese101 commented Sep 17, 2023

If possible the Item Select Query through frappe.db.sql will be changed to use frappe.qb instead as it is way secure and will minimize the possibility of SQL Injection.

if it is not possible to use frappe.qb instead of frappe.db.sql, I will just use SQL placeholders for the item_group and attribute filters.

@yrestom
Copy link
Contributor

yrestom commented Oct 2, 2023

Can you please explain the use case?

@Madcheese101
Copy link
Author

This feature offers the ability to add two more filters to the POS interface to filter the products like the Item Group Filter.

You can enable filtering using two attributes of your choice, for example, you can enable the first attribute filter and set its data to the list of Size attributes.
Also, you have the ability to enable a second filter for another attribute of your choice, like the color.

This feature is disabled by default and can be enabled by the user.

changed from frappe.db.sql (method to get items_data) to frappe.qb for more security and to follow the new direction to push for frappe.qb instead if frappe.db.sql
@Madcheese101
Copy link
Author

Madcheese101 commented Oct 26, 2023

New Commit: changed from frappe.db.sql to frappe.qb

changed from frappe.db.sql (method to get items_data) to frappe.qb for more security and to follow the new direction to push for the frappe.qb api instead of frappe.db.sql

Changed filtering items depending on if they have stock or not through the option (hide unavailable items) in POS profile to be applied as a condition while fetching the data from the database instead of looping through the db result and then filtering.

this results in a better performance when the (Hide Unavailable Items) is enabled.
@Madcheese101
Copy link
Author

New Commit:
changed the hide_unavailable_items logic to work on DB directly

Changed filtering items depending on whether they have stock or not through the option Hide Unavailable Items in POS profile to be applied as a condition while fetching the data from the database instead of looping through the DB result and then filtering.

This results in a better performance when the (Hide Unavailable Items) is enabled.

@Madcheese101
Copy link
Author

If you need more clarification on any of the commits, please let me know.

@yrestom
Copy link
Contributor

yrestom commented Dec 5, 2023

I think we already have this feature but it works in different way,
When the user press on template item he will se a popup window to select the variant item and he can select the needed attributes to filter the items

@Madcheese101
Copy link
Author

Madcheese101 commented Dec 5, 2023

Doesn't the template item filters popup only work if you have show template items set to true?

I didn't know that it works as a filter, to be honest.

I just thought that this feature would be a clean, easy, and straightforward way for managers to set filters on specific attributes that they think are the most important for their use case scenario.

Also, I was planning to refactor/redesign that popup window for better UX.

@yrestom
Copy link
Contributor

yrestom commented Dec 5, 2023

Ok, then I think this PR not necessary any more

@Madcheese101
Copy link
Author

Madcheese101 commented Dec 5, 2023

I think it is a bit inconvenient to have to look for and click on the template item to set some filters for items that you want to search for.

This is not UX friendly, that's why i thought the Show Template Items was supposed to only work when you hide the variant items.

And that's why I came up with this idea, however if you see this as pointless and the Show Template Items option is a filtering method, then this BR should be closed and the current filtering logic NEEDS some SERIOUS refactoring.

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