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: Redisearch with consent (bp) #30539

Merged

Conversation

marination
Copy link
Collaborator

@marination marination commented Apr 1, 2022

Port of #30522
Closes #30559

Can be tested on https://rs-install-1.frappe.cloud/all-products (v13)

Documentation: https://docs.erpnext.com/docs/v13/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
  • 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)
    Screenshot 2022-04-04 at 1 44 35 PM
  • Redisearch will be used if it is installed/loaded AND enabled in settings
  • Item group suggestions in Search is functional:
    2022-04-04 13 48 15
  • 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

- 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 marination changed the title fix: Call Redisearch index creation functions on enabling redisearch in settings feat: Redisearch with consent Apr 4, 2022
@marination marination marked this pull request as ready for review April 4, 2022 08:24
@marination marination changed the title feat: Redisearch with consent feat: Redisearch with consent (bp) Apr 4, 2022
@marination
Copy link
Collaborator Author

Failing tests unrelated

@marination marination merged commit 761f8e0 into frappe:version-13-hotfix Apr 4, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant