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

refactor!: make automatically following documents optional #16030

Merged
merged 28 commits into from
Mar 24, 2022

Conversation

hasnain2808
Copy link
Contributor

@hasnain2808 hasnain2808 commented Feb 17, 2022

Creating content for Document Follow emails is very heavy.
It collects all changes and comments for each document followed by a user

Changes:

  1. Documents will not be followed automatically but there is an option to turn this behavior on (Backwards Compatibility).

image

  1. Minor optimizations on how the users and changes are collected

  2. Limiting document per user to 50. Ordered by the latest document followed. This will make the job execution time to depend only on the number of users and the number of documents they are following

Typical Document Follow Email for reference
image
Docs for Document Follow: https://docs.erpnext.com/docs/v13/user/manual/en/setting-up/email/document-follow

@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #16030 (a8dc432) into develop (833eeff) will decrease coverage by 1.55%.
The diff coverage is 76.08%.

@@             Coverage Diff             @@
##           develop   #16030      +/-   ##
===========================================
- Coverage    56.80%   55.25%   -1.56%     
===========================================
  Files          758      756       -2     
  Lines        67423    67261     -162     
  Branches      5797     5770      -27     
===========================================
- Hits         38297    37162    -1135     
- Misses       25576    26275     +699     
- Partials      3550     3824     +274     
Flag Coverage Δ
server 62.07% <76.08%> (-0.22%) ⬇️
ui-tests 42.97% <ø> (-4.04%) ⬇️

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

@hasnain2808 hasnain2808 changed the title fix: make automatically following documents optional refactor: make automatically following documents optional Feb 21, 2022
@hasnain2808 hasnain2808 changed the title refactor: make automatically following documents optional refactor!: make automatically following documents optional Feb 23, 2022
@surajshetty3416 surajshetty3416 added the tests-failing Automated tests are failing. Please resolve if it is due to the changes in current PR. label Mar 1, 2022
@stale
Copy link

stale bot commented Mar 11, 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 11, 2022
@stale stale bot closed this Mar 14, 2022
@hasnain2808 hasnain2808 reopened this Mar 22, 2022
@stale stale bot removed the inactive label Mar 22, 2022
@hasnain2808 hasnain2808 added tests-failing Automated tests are failing. Please resolve if it is due to the changes in current PR. and removed tests-failing Automated tests are failing. Please resolve if it is due to the changes in current PR. labels Mar 24, 2022
@hasnain2808
Copy link
Contributor Author

hasnain2808 commented Mar 24, 2022

Test Failures caused by: #15964

@hasnain2808 hasnain2808 merged commit fda544f into frappe:develop Mar 24, 2022
@hasnain2808 hasnain2808 removed the tests-failing Automated tests are failing. Please resolve if it is due to the changes in current PR. label Mar 24, 2022
hasnain2808 added a commit to phot0n/frappe that referenced this pull request Mar 25, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 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

4 participants