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

Faceted search #44

Merged
merged 65 commits into from
Aug 12, 2024
Merged

Faceted search #44

merged 65 commits into from
Aug 12, 2024

Conversation

agritheory
Copy link
Owner

No description provided.

@agritheory
Copy link
Owner Author

agritheory commented Dec 11, 2023

@Alchez Can you branch and pick up changes for list view integration? That should be a PR to this branch. Main integration points should be inside setFilterValues in FacetSearch.vue. Which we should abstract to api.js anyway.

@agritheory agritheory changed the title Faceted search Draft: Faceted search Jan 4, 2024
@agritheory
Copy link
Owner Author

agritheory commented Feb 26, 2024

To Dos as of 2/26/24:

  • Make views configurable in Inventory Tools settings - all-products, Item List View, via frappe.boot Load inventory_tools settings into frappe.boot #73
  • Document and manually test Specification configuration workflow
  • Add helper on Item Group (to generate Specification Values for Items in that group)
  • Add helper on Item to edit that Item's Specification Values

@agritheory
Copy link
Owner Author

@bhattdevarsh This looks great! A couple more things to incorporate:

  • Reduce the width of the scrollbar on Firefox (this looks like a default width and I think we want it to be 75 or 80%)
  • Let's add a collapse caret on the top right

image

@bhattdevarsh
Copy link
Collaborator

  • Added collapsible div

  • Managed Icon, onclick event and cursor pointer

  • Added key value to searchComponent called visible
    filter_scroller_and_collapsable

  • Made possible changes to customize scrollbar on Firefox
    firefox_Filter_Scroller

@agritheory agritheory linked an issue Apr 15, 2024 that may be closed by this pull request
@agritheory agritheory marked this pull request as draft April 15, 2024 16:18
@agritheory agritheory changed the title Draft: Faceted search Faceted search Apr 15, 2024
package.json Show resolved Hide resolved
inventory_tools/docs/faceted_search.md Outdated Show resolved Hide resolved
inventory_tools/docs/faceted_search.md Outdated Show resolved Hide resolved
inventory_tools/docs/faceted_search.md Outdated Show resolved Hide resolved
inventory_tools/docs/faceted_search.md Outdated Show resolved Hide resolved
inventory_tools/docs/faceted_search.md Outdated Show resolved Hide resolved
inventory_tools/docs/faceted_search.md Outdated Show resolved Hide resolved
inventory_tools/docs/faceted_search.md Outdated Show resolved Hide resolved
inventory_tools/docs/faceted_search.md Show resolved Hide resolved
inventory_tools/docs/faceted_search.md Outdated Show resolved Hide resolved
@bhattdevarsh bhattdevarsh requested a review from Alchez April 23, 2024 10:57
Alchez
Alchez previously requested changes Apr 24, 2024
inventory_tools/inventory_tools/faceted_search.py Outdated Show resolved Hide resolved
inventory_tools/public/js/faceted_search/FacetedSearch.vue Outdated Show resolved Hide resolved
@agritheory
Copy link
Owner Author

agritheory commented Apr 24, 2024

Still required for this to be considered complete:

To Dos as of 2/26/24:

  • Make views configurable in Inventory Tools settings - all-products, Item List View, via frappe.boot Load inventory_tools settings into frappe.boot #73
  • Document and manually test Specification configuration workflow
  • Add helper on Item Group (to generate Specification Values for Items in that group)
  • Add helper on Item to edit that Item's Specification Values

@bhattdevarsh Can you add these last two features?

@bhattdevarsh
Copy link
Collaborator

@agritheory @Alchez

  • Add helper on Item Group (to generate Specification Values for Items in that group)

I am thinking of a solution where we can use similar approach as helper on Item, Having a button called "Generate Specification Values" under Item Group and on click of that we fetch specification and attributes created for that group. Some Attributes can have some field mapped with it and some may not. For the mapped field attributes we can write a logic that auto-fetch the values from group items and for remaining we let user type it manually.

Attaching the image that demonstrates how the dialogue box may look like. please add/suggest necessary changes.

image

@agritheory
Copy link
Owner Author

@bhattdevarsh I was thinking about this too and I think Item Group is actually the wrong place to put this, it should be on the specification itself. I like your approach and I think it makes sense to reuse or modularize the existing modal.

Comment on lines 221 to 223
for value in specification.value.split(","):
#TODO: identify and attach reference_doctype & reference_name
frappe.get_doc({
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are losing information like Item(reference_doctype) and Item name(reference_name) while creating bulk specification values. We may need design changes to get expected results.

image

@bhattdevarsh
Copy link
Collaborator

Previous commit carries solution to this issue-> #76

agritheory and others added 2 commits May 13, 2024 16:45
Automatically generated by python-semantic-release
@agritheory agritheory requested a review from bhattdevarsh May 21, 2024 15:18
@agritheory agritheory marked this pull request as ready for review May 23, 2024 14:47
@agritheory
Copy link
Owner Author

It does not appear as though the configuration for app/website visibility is working, filters do not appear to being set in the list view and the styling I think is not getting pulled in.

image

This feature is getting very lose but it isn't quite there yet. CC @Alchez @bhattdevarsh

Comment on lines +134 to +163
def convert_to_epoch(date):
tzname = time.tzname if isinstance(time.tzname, (int, str)) else time.tzname[0]

try:
tz = timezone(tzname)
except UnknownTimeZoneError:
# default to beginning of epoch
return

d = datetime.datetime.now(tz) # or some other local date
utc_offset = d.utcoffset()
if utc_offset:
utc_offset_seconds = utc_offset.total_seconds()
offset_d = (
get_datetime(date) - datetime.timedelta(hours=12, seconds=int(utc_offset_seconds))
) - get_datetime("1970-01-01")
return offset_d.total_seconds()
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

@agritheory @bhattdevarsh The older function was failing for me locally because the tzname variable was defaulting to "IST", but the timezone module couldn't parse that string. Maybe we could find an easier way of figuring out the epoch time from a given string?

filters=filters,
pluck="reference_name",
)
specification_items.update(item_codes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@agritheory @bhattdevarsh The system was generating a list of sets which was causing the string display in the UI to become too long. I've changed the data type to a simple set of strings and the return value to a simple list of strings.

Let me know if that's the incorrect approach.

inventory_tools/public/js/faceted_search/FacetedSearch.vue Outdated Show resolved Hide resolved
@Alchez
Copy link
Collaborator

Alchez commented May 24, 2024

@agritheory

It does not appear as though the configuration for app/website visibility is working, filters do not appear to being set in the list view

I've added a commit that should allow the filters to work, but I'm seeing weird UI issues with the applied filters that I've noted in a comment above.

and the styling I think is not getting pulled in.

I couldn't see any stylesheets in the app, and I think all of it is coming from the vue files directly. I think they might just be incompatible or are being overridden?

HKuz and others added 3 commits May 24, 2024 07:58
* test: add test for alternative workstation

* feat: make alternative workstations configurable

* fix: uncomment js code for testing

* feat: search alternative workstation names

* refactor: pop filters that cause error for Workstation

* chore: update override commit hash

---------

Co-authored-by: Heather Kusmierz <[email protected]>
@agritheory agritheory requested a review from Alchez June 13, 2024 13:39
Copy link

Coverage

Coverage Report
FileStmtsMissCoverMissing
inventory_tools
   __init__.py50100% 
   customize.py24240%4–52
   hooks.py200100% 
inventory_tools/inventory_tools
   __init__.py00100% 
   boot.py660%4–11
   faceted_search.py13010321%18–69, 85–119, 126–154, 157–160, 165–215, 237, 246–309
inventory_tools/inventory_tools/doctype
   __init__.py00100% 
inventory_tools/inventory_tools/doctype/inventory_tools_settings
   __init__.py00100% 
   inventory_tools_settings.py593541%16–17, 24–45, 60–65, 73–78, 85–91
   test_inventory_tools_settings.py30100% 
inventory_tools/inventory_tools/doctype/purchase_invoice_subcontracting_detail
   __init__.py00100% 
   purchase_invoice_subcontracting_detail.py330%5–9
inventory_tools/inventory_tools/doctype/purchase_order_subcontracting_detail
   __init__.py00100% 
   purchase_order_subcontracting_detail.py30100% 
inventory_tools/inventory_tools/doctype/specification
   __init__.py00100% 
   specification.py20612042%19–21, 25–37, 43, 57–58, 73–122, 130, 133, 139–143, 151–153, 158–162, 167–169, 174–175, 183–238, 243–268, 276, 292, 310, 330–341, 348–364
   test_specification.py30100% 
inventory_tools/inventory_tools/doctype/specification_attribute
   __init__.py00100% 
   specification_attribute.py30100% 
inventory_tools/inventory_tools/doctype/specification_value
   __init__.py00100% 
   specification_value.py30100% 
   test_specification_value.py30100% 
inventory_tools/inventory_tools/doctype/subcontracting_default
   __init__.py00100% 
   subcontracting_default.py330%5–9
inventory_tools/inventory_tools/overrides
   job_card.py17288%25, 32
   purchase_order.py14810032%22, 81–84, 93–101, 112–131, 136–154, 159–222, 234–240, 253, 262–264, 269–274
   sales_order.py24196%46
   stock_entry.py781976%77–78, 81, 87, 96, 104, 162, 168, 172, 180–200
   uom.py631084%12, 25–27, 48–49, 88, 95, 114, 118
   warehouse.py431663%12, 17, 36–37, 40, 54–68
   work_order.py25121614%26–29, 32–37, 40–42, 45–46, 49–50, 57–58, 65–93, 102–112, 125–158, 168–181, 191–232, 240–270, 276–289, 293–316, 320–357, 365–380, 385–471, 476–498, 512
   workstation.py532258%16–20, 27–40, 43–56, 91–93, 104
inventory_tools/inventory_tools/report
   __init__.py00100% 
inventory_tools/inventory_tools/report/manufacturing_capacity
   __init__.py00100% 
   manufacturing_capacity.py805531%11–12, 16, 84–115, 127–133, 214–296, 311–316, 324–338
inventory_tools/inventory_tools/report/material_demand
   __init__.py00100% 
   material_demand.py148994%18, 207–208, 227, 240, 248, 258, 274, 315
inventory_tools/inventory_tools/report/quotation_demand
   __init__.py00100% 
   quotation_demand.py63494%14, 48, 164, 184
inventory_tools/inventory_tools/report/specification
   __init__.py00100% 
   specification.py26260%4–78
inventory_tools/tests
   conftest.py24292%29–30
   test_alternative_workstation.py140100% 
   test_faceted_search.py401855%29–62
   test_manufacturing_capacity.py500100% 
   test_material_demand.py117893%47, 92, 129, 136–138, 185, 233
   test_overproduction.py970100% 
   test_quotation_demand_report.py71199%43
   test_uom.py290100% 
   test_warehouse_path.py120100% 
TOTAL192280358% 

- styles
- API / data creation
- tests
wrong specification
@agritheory
Copy link
Owner Author

@Alchez Can you revisit the listview filter setting logic? Perhaps an API has changed. I recommend a clean install and then running bench execute 'inventory_tools.tests.setup.create_demo_specification_values' to setup the appropriate data.

@Alchez
Copy link
Collaborator

Alchez commented Aug 5, 2024

@agritheory I was able to get the filters to run, but it seems like a couple of tests are still failing (CI is reporting incorrectly for some reason). Also, we should get on a call to confirm the final workflows before a merge.

@agritheory
Copy link
Owner Author

@Alchez /all-products is now filtering correctly, though the sort order feature is still broken. I am having an issue in the listview where the filter values are not being set correctly: there's a leading comma, and when de/reselecting the "brand" attribute, It doesn't seems to respond reasonably. Perhaps this the debounce function?

image

I think we need to add a few compatibility features here:
When deselecting (no filters are set), the filters are not cleared.
image

When you click "Clear Filters" it does not reset the Faceted Search selections. We can listen on the clear filters click, I think, but we'd need to add a reset API to attribute filters as well.
image

@Alchez
Copy link
Collaborator

Alchez commented Aug 6, 2024

@agritheory I've fixed up the bugs you mentioned. Can you give this another test please?

@agritheory agritheory merged commit 398d970 into version-15 Aug 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Faceted Search
4 participants