feat: Redisearch with consent #30522

marination · 2022-03-31T11:00:34Z

Can be tested on https://rs-install-1.frappe.cloud/all-products (v13) (cherry picked from this PR)

Documentation: https://docs.erpnext.com/docs/v14/user/manual/en/e_commerce/e_commerce_search

Issues:

App Install breakage

App install would break due to stray methods in the redisearch_utils.py file being executed before migrate

Installing erpnext...
Updating DocTypes for erpnext	    : [======================================= ] 99%Traceback (most recent call last):
  File "/home/frappe/frappe-bench/apps/frappe/frappe/commands/site.py", line 190, in install_app
    _install_app(app, verbose=context.verbose)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/installer.py", line 162, in install_app
    sync_for(name, force=True, sync_everything=True, verbose=verbose, reset_permissions=True)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/sync.py", line 70, in sync_for
    import_file_by_path(doc_path, force=force, ignore_version=True, reset_permissions=reset_permissions)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/modules/import_file.py", line 137, in import_file_by_path
    path=path,
  File "/home/frappe/frappe-bench/apps/frappe/frappe/modules/import_file.py", line 257, in import_doc
    doc.insert()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 268, in insert
    self.run_post_save_methods()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 1003, in run_post_save_methods
    self.run_method("on_update")
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 867, in run_method
    out = Document.hook(fn)(self, *args, **kwargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 1160, in composer
    return composed(self, method, *args, **kwargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 1143, in runner
    add_to_return_value(self, fn(self, *args, **kwargs))
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 861, in <lambda>
    fn = lambda self, *args, **kwargs: getattr(self, method)(*args, **kwargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/core/doctype/doctype/doctype.py", line 326, in on_update
    self.run_module_method("on_doctype_update")
  File "/home/frappe/frappe-bench/apps/frappe/frappe/core/doctype/doctype/doctype.py", line 380, in run_module_method
    module = load_doctype_module(self.name, self.module)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/modules/utils.py", line 205, in load_doctype_module
    doctype_python_modules[key] = frappe.get_module(module_name)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 975, in get_module
    return importlib.import_module(modulename)
  File "/usr/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/frappe/frappe-bench/apps/erpnext/erpnext/e_commerce/doctype/website_item/website_item.py", line 13, in <module>
    from erpnext.e_commerce.doctype.item_review.item_review import get_item_reviews
  File "/home/frappe/frappe-bench/apps/erpnext/erpnext/e_commerce/doctype/item_review/item_review.py", line 13, in <module>
    from erpnext.e_commerce.doctype.e_commerce_settings.e_commerce_settings import (
  File "/home/frappe/frappe-bench/apps/erpnext/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.py", line 9, in <module>
    from erpnext.e_commerce.redisearch_utils import (
  File "/home/frappe/frappe-bench/apps/erpnext/erpnext/e_commerce/redisearch_utils.py", line 209, in <module>
    define_autocomplete_dictionary()
  File "/home/frappe/frappe-bench/apps/erpnext/erpnext/e_commerce/redisearch_utils.py", line 38, in wrapper
    func = function(*args, **kwargs)
  File "/home/frappe/frappe-bench/apps/erpnext/erpnext/e_commerce/redisearch_utils.py", line 152, in define_autocomplete_dictionary
    'show_categories_in_search_autocomplete'
  File "/home/frappe/frappe-bench/apps/frappe/frappe/database/database.py", line 570, in get_single_value
    df = frappe.get_meta(doctype).get_field(fieldname)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 917, in get_meta
    return frappe.model.meta.get_meta(doctype, cached=cached)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/meta.py", line 37, in get_meta
    meta = Meta(doctype)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/meta.py", line 83, in __init__
    super(Meta, self).__init__("DocType", doctype)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 113, in __init__
    self.load_from_db()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/meta.py", line 88, in load_from_db
    super(Meta, self).load_from_db()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 156, in load_from_db
    frappe.throw(_("{0} {1} not found").format(_(self.doctype), self.name), frappe.DoesNotExistError)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 439, in throw
    msgprint(msg, raise_exception=exc, title=title, indicator='red', is_minimizable=is_minimizable, wide=wide, as_list=as_list)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 418, in msgprint
    _raise_exception()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 372, in _raise_exception
    raise raise_exception(msg)
frappe.exceptions.DoesNotExistError: DocType E Commerce Settings not found
An error occurred while installing erpnext:
DocType E Commerce Settings not found

Use Redisearch with consent

A system could have had redisearch installed for other reasons (not for erpnext)
It is wrong to assume that if redisearch is just installed we must use it for searches
Also if something goes wrong with redisearch there should be a way to disable it temporarily

Product category results in Search was broken

The Product Category results in search had no redirect attached to it.
This is because the redisearch only returned the item group names, no metadata
Also, item group suggestion generation was redundantly done again and again

Useless bool return values from functions and absence of exception handling

Fix:

App install (tried on FC, ERPNext installs flawlessly after installed RS on bench. Thats how the site above has ERPNext)
Once Redisearch is installed, the Item Search Settings will not be read only. Users must then enable Redisearch in the Settings. This can be disabled too (normal db search will be used)
Redisearch will be used if it is installed/loaded AND enabled in settings
Item group suggestions in Search is functional:
Added error logging for exceptions and left comments where we explicitly ignore the exception
Item Group suggestions are separately generated with payload(extra info) and weightage as well
Minor code cleanup

Skipping tests because cannot write tests as it needs a script to install RS. Will add during app separation

…in settings

codecov · 2022-03-31T11:24:57Z

Codecov Report

Merging #30522 (7e207c8) into develop (901d8ea) will increase coverage by 0.05%.
The diff coverage is 69.56%.

❗ Current head 7e207c8 differs from pull request most recent head 97e3a85. Consider uploading reports for the commit 97e3a85 to get more accurate results

@@             Coverage Diff             @@
##           develop   #30522      +/-   ##
===========================================
+ Coverage    60.90%   60.95%   +0.05%     
===========================================
  Files         1082     1083       +1     
  Lines        69078    69120      +42     
===========================================
+ Hits         42071    42135      +64     
+ Misses       27007    26985      -22

Impacted Files	Coverage Δ
erpnext/templates/pages/product_search.py	`0.00% <0.00%> (ø)`
...doctype/e_commerce_settings/e_commerce_settings.py	`64.76% <37.50%> (-2.25%)`	⬇️
erpnext/e_commerce/redisearch_utils.py	`34.95% <100.00%> (+0.53%)`	⬆️
...payroll/doctype/income_tax_slab/income_tax_slab.py	`83.33% <0.00%> (-16.67%)`	⬇️
...rpnext/accounts/doctype/shareholder/shareholder.py	`80.00% <0.00%> (-10.00%)`	⬇️
...saction/incorrect_balance_qty_after_transaction.py	`90.69% <0.00%> (-9.31%)`	⬇️
...pnext/accounts/report/gross_profit/gross_profit.py	`76.95% <0.00%> (-4.53%)`	⬇️
erpnext/stock/doctype/warehouse/warehouse.py	`84.42% <0.00%> (-2.46%)`	⬇️
.../report/bom_operations_time/bom_operations_time.py	`89.13% <0.00%> (-2.18%)`	⬇️
erpnext/stock/reorder_item.py	`75.21% <0.00%> (-1.71%)`	⬇️
... and 34 more

- Separate Item group and Item autocomplete dict definition - Add payload along with Item group, containing namke and route - Pass weightage while defining item group autocomplete dict (auto sort) - Use payload while getting results for categories in search - Remove check to show categories, always show - Search fields mandatory if reidsearch enabled - Code separation (rough)

- Function to handle RS exceptions (create log and raise error) - Handle `ResponseError` where it is anticipated - Misc: Better variables

- If score 0 is inserted into suggestions, RS does not consider that suggestion

marination · 2022-04-04T11:04:30Z

Failed check failing at uploading code cov data. Tests are passing

marination · 2022-04-04T13:53:41Z

failing test unrelated

fix: Call Redisearch index creation functions on enabling redisearch …

7e207c8

…in settings

github-actions bot added ecommerce needs-tests This PR needs automated unit-tests. labels Mar 31, 2022

marination mentioned this pull request Apr 1, 2022

feat: Redisearch with consent (bp) #30539

Merged

marination added 7 commits April 1, 2022 18:47

fix: Better Exception Handling and vaeiabl naming

ea036e4

- Function to handle RS exceptions (create log and raise error) - Handle `ResponseError` where it is anticipated - Misc: Better variables

fix: Convert payload to string before adding to autocompleter

97e3a85

fix: Add default score of 1 to Item Group Autocompleter

7ef1ccb

- If score 0 is inserted into suggestions, RS does not consider that suggestion

fix: Payload incorrect data (pass item_group.name)

3445682

chore: Add TODOs(perf) for Redisearch Query

397b46a

Merge branch 'develop' into redisearch-app-install

bfb2fbe

marination changed the title ~~fix: Call Redisearch index creation functions on enabling redisearch in settings~~ feat: Redisearch with consent Apr 4, 2022

marination marked this pull request as ready for review April 4, 2022 08:26

marination removed the needs-tests This PR needs automated unit-tests. label Apr 4, 2022

Merge branch 'develop' into redisearch-app-install

569eff4

marination added the needs-docs label Apr 4, 2022

Merge branch 'develop' into redisearch-app-install

0a592e0

marination removed the needs-docs label Apr 4, 2022

marination merged commit 3c3aca9 into frappe:develop Apr 4, 2022

deepeshgarg007 mentioned this pull request Jul 18, 2022

List of V14 features & enhancements #31620

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Redisearch with consent #30522

feat: Redisearch with consent #30522

marination commented Mar 31, 2022 •

edited

Loading

codecov bot commented Mar 31, 2022 •

edited

Loading

marination commented Apr 4, 2022

marination commented Apr 4, 2022

feat: Redisearch with consent #30522

feat: Redisearch with consent #30522

Conversation

marination commented Mar 31, 2022 • edited Loading

Issues:

Fix:

codecov bot commented Mar 31, 2022 • edited Loading

Codecov Report

marination commented Apr 4, 2022

marination commented Apr 4, 2022

marination commented Mar 31, 2022 •

edited

Loading

codecov bot commented Mar 31, 2022 •

edited

Loading