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: add translated search doctypes to hooks #16197

Merged
merged 9 commits into from Mar 23, 2022

Conversation

barredterra
Copy link
Collaborator

@barredterra barredterra commented Mar 3, 2022

In search.py it was hardcoded that DocType and Role get translated before matching against the search text. This way, a user can type in his local language and still see correct results.

This feature is useful for other DocTypes as well. The criterion would be: there is a small, fairly static number of records, so that the performance impact of translating all names first is not too bad.

This PR adds a hook translated_search_doctypes that determines which DocType names get translated before search.

I also added Country to translated_search_doctypes for frappe. The link to Country is frequently used in Address, but until now there was no way to use it in the local language. There are ~70% less Countries than DocTypes (including ERPNext), so the performance should be fine.

ERPNext could, for example, add the Gender DocType to this hook. As there are very few genders, translating them is fast and improves the UX.

Docs: https://frappeframework.com/docs/v13/user/en/python-api/hooks/edit?wiki_page_patch=b4d7c8d6fc

@barredterra barredterra requested a review from a team as a code owner March 3, 2022 22:01
@barredterra barredterra requested review from hasnain2808 and removed request for a team March 3, 2022 22:01
@barredterra
Copy link
Collaborator Author

@Mergifyio backport version-13-hotfix

@mergify
Copy link
Contributor

mergify bot commented Mar 3, 2022

backport version-13-hotfix

🟠 Waiting for conditions to match

  • merged [:pushpin: backport requirement]

@codecov
Copy link

codecov bot commented Mar 3, 2022

Codecov Report

Merging #16197 (d02a3f8) into develop (64efc6b) will decrease coverage by 1.93%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #16197      +/-   ##
===========================================
- Coverage    56.74%   54.81%   -1.94%     
===========================================
  Files          758      758              
  Lines        67344    67337       -7     
  Branches      5795     5795              
===========================================
- Hits         38214    36910    -1304     
- Misses       25603    26882    +1279     
- Partials      3527     3545      +18     
Flag Coverage Δ
server 59.17% <100.00%> (-3.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@barredterra barredterra changed the title feat: add translated serach doctypes to hooks feat: add translated search doctypes to hooks Mar 3, 2022
@stale
Copy link

stale bot commented Mar 14, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Mar 14, 2022
@barredterra
Copy link
Collaborator Author

@hasnain2808 I'd appreciate your feedback on this PR, if you can find any time. Thanks in advance! :)

@stale
Copy link

stale bot commented Mar 22, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Mar 22, 2022
@sagarvora
Copy link
Member

sagarvora commented Mar 22, 2022

ERPNext could, for example, add the Gender DocType to this hook. As there are very few genders, translating them is fast and improves the UX.

Gender is in Frappe. Maybe we should add it by default? Salutation as well?

Copy link
Member

@sagarvora sagarvora left a comment

Choose a reason for hiding this comment

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

Just a few minor suggestions and thoughts. Rest LGTM.

frappe/sessions.py Outdated Show resolved Hide resolved
frappe/desk/search.py Show resolved Hide resolved
frappe/utils/boilerplate.py Outdated Show resolved Hide resolved
Co-authored-by: Sagar Vora <sagar@resilient.tech>
@barredterra barredterra mentioned this pull request Mar 23, 2022
barredterra and others added 2 commits March 23, 2022 11:20
Co-authored-by: Sagar Vora <sagar@resilient.tech>
@mergify mergify bot merged commit f80a16e into frappe:develop Mar 23, 2022
@mergify
Copy link
Contributor

mergify bot commented Mar 23, 2022

backport version-13-hotfix

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Mar 23, 2022
In `search.py` it was hardcoded that **DocType** and **Role** get translated before matching against the search text. This way, a user can type in his local language and still see correct results.

This feature is useful for other DocTypes as well. The criterion would be: there is a small, fairly static number of records, so that the performance impact of translating all names first is not too bad.

This PR adds a hook `translated_search_doctypes` that determines which DocType names get translated before search.

I also added **Country** to `translated_search_doctypes` for frappe. The link to **Country** is frequently used in **Address**, but until now there was no way to use it in the local language. There are ~70% less Countries than DocTypes (including ERPNext), so the performance should be fine.

ERPNext could, for example, add the **Gender** DocType to this hook. As there are very few genders, translating them is fast and improves the UX.

Docs: https://frappeframework.com/docs/v13/user/en/python-api/hooks/edit?wiki_page_patch=b4d7c8d6fc
(cherry picked from commit f80a16e)
@barredterra barredterra deleted the translated-search-doctypes branch March 29, 2022 10:51
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants