Skip to content

[WIP] email share notification should be done in the background#32125

Closed
sharidas wants to merge 1 commit intomasterfrom
background-share-notification
Closed

[WIP] email share notification should be done in the background#32125
sharidas wants to merge 1 commit intomasterfrom
background-share-notification

Conversation

@sharidas
Copy link
Copy Markdown
Contributor

@sharidas sharidas commented Jul 23, 2018

The share notification should be done as background
process. This would help for groups with large users
to send notifications and emails.

Signed-off-by: Sujith H sharidasan@owncloud.com

Description

Related Issue

  • Fixes <issue_link>

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

@sharidas sharidas self-assigned this Jul 23, 2018
@sharidas sharidas force-pushed the background-share-notification branch from d265547 to 2c998b5 Compare July 24, 2018 11:34
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 24, 2018

Codecov Report

Merging #32125 into master will decrease coverage by <.01%.
The diff coverage is 25%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #32125      +/-   ##
============================================
- Coverage     63.96%   63.96%   -0.01%     
- Complexity    18598    18600       +2     
============================================
  Files          1172     1172              
  Lines         69833    69837       +4     
  Branches       1267     1267              
============================================
+ Hits          44669    44670       +1     
- Misses        24795    24798       +3     
  Partials        369      369
Flag Coverage Δ Complexity Δ
#javascript 52.81% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.23% <25%> (-0.01%) 18600 <2> (+2)
Impacted Files Coverage Δ Complexity Δ
lib/private/Notification/Manager.php 95.28% <0%> (-2.78%) 44 <2> (+2)
...iles_sharing/lib/Service/NotificationPublisher.php 89.65% <100%> (+0.18%) 15 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0233b72...1c10ddc. Read the comment docs.

@sharidas sharidas force-pushed the background-share-notification branch from 2c998b5 to 9b02008 Compare July 24, 2018 11:55
@sharidas
Copy link
Copy Markdown
Contributor Author

The acceptance test has failed due to the change. Looking for the fix.

@sharidas sharidas force-pushed the background-share-notification branch from 9b02008 to a5b37ba Compare July 30, 2018 14:15
@sharidas sharidas changed the title [WIP] The share notification should be done in the background [WIP] email share notification should be done in the background Jul 30, 2018
The share notification through email should be done
in the background. This change helps to split the
notification of users and email as separate tasks.
Its always better to have email notification as
background job.

Signed-off-by: Sujith H <sharidasan@owncloud.com>
@sharidas sharidas force-pushed the background-share-notification branch from a5b37ba to 1c10ddc Compare July 30, 2018 16:28
@PVince81 PVince81 assigned sharidasan and unassigned sharidas Oct 23, 2018
@DeepDiver1975
Copy link
Copy Markdown
Member

As discussed in #35777 the master branch will from now on hold the ownCloud 10 codebase.

This PR targetted ownCloud 11 which is postponed to a far distant future.

Because of that I'm closing this PR and kindly ask you to re-submit this PR in a few days.

Thanks a lot for your patience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants