-
Notifications
You must be signed in to change notification settings - Fork 14
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
Faceted search #44
Conversation
@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 |
To Dos as of 2/26/24:
|
@bhattdevarsh This looks great! A couple more things to incorporate:
|
Still required for this to be considered complete:
@bhattdevarsh Can you add these last two features? |
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. |
@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. |
for value in specification.value.split(","): | ||
#TODO: identify and attach reference_doctype & reference_name | ||
frappe.get_doc({ |
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.
Previous commit carries solution to this issue-> #76 |
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. This feature is getting very lose but it isn't quite there yet. CC @Alchez @bhattdevarsh |
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 |
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.
@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) |
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.
@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.
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.
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? |
Co-authored-by: Rohan Bansal <[email protected]>
* 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]>
66282c2
to
6ec5867
Compare
Coverage Report
|
- styles - API / data creation - tests
wrong specification
b697a32
to
fc27225
Compare
@Alchez Can you revisit the listview filter setting logic? Perhaps an API has changed. I recommend a clean install and then running |
@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. |
@Alchez I think we need to add a few compatibility features here: 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. |
@agritheory I've fixed up the bugs you mentioned. Can you give this another test please? |
No description provided.