Sendmail command #13079

zhiburt · 2020-10-08T22:26:57Z

Implements a sendmail command which sends a message to all users that are present in db.

Tented to close #12996

I am not versed in a code base so there may be some issues 😞 .

Thank you.

Usefull to have when you need to be confident that message was sent.

codecov-io · 2020-10-08T23:18:12Z

Codecov Report

Merging #13079 into master will increase coverage by 0.07%.
The diff coverage is 1.16%.

@@            Coverage Diff             @@
##           master   #13079      +/-   ##
==========================================
+ Coverage   42.05%   42.12%   +0.07%     
==========================================
  Files         681      687       +6     
  Lines       75121    75632     +511     
==========================================
+ Hits        31594    31862     +268     
- Misses      38372    38547     +175     
- Partials     5155     5223      +68

Impacted Files	Coverage Δ
cmd/admin.go	`0.00% <ø> (ø)`
cmd/cmd.go	`17.85% <0.00%> (-13.40%)`	⬇️
cmd/mailer.go	`0.00% <0.00%> (ø)`
modules/private/mail.go	`0.00% <0.00%> (ø)`
routers/private/mail.go	`0.00% <0.00%> (ø)`
routers/private/internal.go	`88.46% <100.00%> (+0.46%)`	⬆️
modules/queue/helper.go	`47.36% <0.00%> (-14.71%)`	⬇️
routers/init.go	`52.06% <0.00%> (-14.61%)`	⬇️
modules/queue/setting.go	`31.25% <0.00%> (-14.43%)`	⬇️
services/gitdiff/gitdiff.go	`67.39% <0.00%> (-7.50%)`	⬇️
... and 72 more

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 e374bb7...af3cff6. Read the comment docs.

techknowlogick

Awesome!! Thanks so much for the PR :) We ❤️ new contributors.

A couple of bits of feedback:

Could you split this off into a separate file (admin.go is getting a bit long at this point), perhaps cmd/mailer.go?
Can you add checks that if either title or content is empty then error out?
Could you add a confirmation step (and perhaps a --force flag to bypass confirmation) so that emails don't accidentally get sent out.

cmd/admin.go

zhiburt · 2020-10-09T09:40:20Z

Thank you for your message @techknowlogick.

According to your second point I could image that content/body may be empty what do you think?

cmd/admin.go

cmd/mailer.go

techknowlogick · 2020-10-11T22:09:17Z

Perhaps you could add a warning when user is asked to confirm they want to send mail when subject or content is empty instead of my original suggestion of stopping send mail?

Print waring if it's empty or haven't been set up. The warning will be skiped if there's a `--force` flag.

zhiburt · 2020-10-12T14:23:08Z

Just to notice if --force is passed warning will not be printed.
And I left title as required just because an empty email makes not a lot of sense?

Co-authored-by: 6543 <[email protected]>

cmd/mailer.go

…ail_command

IterateUsers uses batching by default. Signed-off-by: Maxim Zhiburt <[email protected]>

…mail_command

Signed-off-by: Maxim Zhiburt <[email protected]>

cmd/mailer.go

Signed-off-by: Maxim Zhiburt <[email protected]>

zeripath · 2020-10-19T12:38:36Z

Sorry to jump in so late.

We need to decide what type of command this is.

If it is intended to communicate with a running Gitea we should create and use an internal api to call and communicate with the running server.

If, as it appears, it is intended not to communicate with a running Gitea you could have used the already available Async methods and just ran flush queues at the end of it.

Signed-off-by: Maxim Zhiburt <[email protected]>

zhiburt · 2020-10-19T17:20:03Z

If, as it appears, it is intended not to communicate with a running Gitea you could have used the already available Async methods and just ran flush queues at the end of it.

Thanks for the hint over Flush function

We need to decide what type of command this is.

If it is intended to communicate with a running Gitea we should create and use an internal api to call and communicate with the running server.

We could support 2 types of calls probably, which could be determined by a flag.

Signed-off-by: Maxim Zhiburt <[email protected]>

zhiburt · 2020-10-22T17:38:30Z

@lunny

I think we should do that via internal API but not push to the queue directly as @zeripath said.

@zeripath

If it is intended to communicate with a running Gitea we should create and use an internal api to call and communicate with the running server.

I've started looking at the API and as I see to use an API as an admin we need to authenticate yourself right?
So it would increase an amount of arguments at the end of the day maybe there's no reason to?

And why actually you consider here it's appropriate to be attached to a running Gitea where others admin commands doesn't what is the benefit?

And if we expose such API in my opinion it just make more sense to provide a page with a form for this purpose and that is it.

cmd/admin.go

Co-authored-by: 6543 <[email protected]>

lunny · 2020-10-23T11:17:31Z

If the configuration sets the mailer queue as levelqueue, the command will fail when start, except queue is a network service.
But if we always use an internal API to do that, it will always work.
The internal API is in /api/v1/internal which can only be visit via the token saved in app.ini. It has no documentation and maybe changed if needed.

…mail_command

zhiburt · 2020-10-23T21:05:44Z

Thanks for the clarification.

So I've updated the PR to use a private API

techknowlogick · 2020-10-24T20:37:54Z

🚀

…s-stored-in-email-address-table * origin/master: [UI] Hide consecutive additions and removals of the same label (go-gitea#13315) [skip ci] Updated translations via Crowdin Fix send mail (go-gitea#13312) [skip ci] Updated translations via Crowdin Deny wrong pull (go-gitea#13308) Group Label Changed Comments in timeline (go-gitea#13304) [skip ci] Updated translations via Crowdin Attempt to handle unready PR in tests (go-gitea#13305) go-gitea#12897 - add mastodon provider (go-gitea#13293) [skip ci] Updated translations via Crowdin Fix Storage mapping (go-gitea#13297) Update Mirror IsEmpty status on synchronize (go-gitea#13185) Fix bug isEnd detection on getIssues/getPullRequests (go-gitea#13299) systemd service: Add commented PATH environment option for Git prefix (go-gitea#13170) Sendmail command (go-gitea#13079) Various UI and arc-green fixes (go-gitea#13291)

zhiburt added 2 commits October 9, 2020 01:21

Add SendSync method

016b35b

Usefull to have when you need to be confident that message was sent.

Add sendmail command

538d0db

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 8, 2020

techknowlogick reviewed Oct 9, 2020

View reviewed changes

techknowlogick added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Oct 9, 2020

techknowlogick added this to the 1.14.0 milestone Oct 9, 2020

lafriks reviewed Oct 9, 2020

View reviewed changes

cmd/admin.go Outdated Show resolved Hide resolved

zhiburt added 4 commits October 9, 2020 11:54

add checks that if either title or content is empty then error out

8ecbcc7

Add a confirmation step

5c0524a

Add --force option to bypass confirm step

2abad3e

Move implementation of runSendMail to a different file

9c98e13

Add copyrighting comment

a750b18

6543 reviewed Oct 11, 2020

View reviewed changes

cmd/admin.go Outdated Show resolved Hide resolved

6543 reviewed Oct 11, 2020

View reviewed changes

cmd/mailer.go Outdated Show resolved Hide resolved

Make content optional

3cb7d18

Print waring if it's empty or haven't been set up. The warning will be skiped if there's a `--force` flag.

Fix import style

d3723b5

Co-authored-by: 6543 <[email protected]>

adelowo reviewed Oct 15, 2020

View reviewed changes

cmd/mailer.go Outdated Show resolved Hide resolved

zhiburt added 4 commits October 15, 2020 23:54

Merge branch 'master' of https://github.com/go-gitea/gitea into sendm…

087129c

…ail_command

Use batch when getting all users

2ed24b5

IterateUsers uses batching by default. Signed-off-by: Maxim Zhiburt <[email protected]>

Merge branch 'sendmail_command' of github.com:zhiburt/gitea into send…

6f4b38d

…mail_command

Send emails one by one instead of as one chunck

59a1bee

Signed-off-by: Maxim Zhiburt <[email protected]>

adelowo reviewed Oct 16, 2020

View reviewed changes

cmd/mailer.go Outdated Show resolved Hide resolved

Send messages concurantly

f5a108d

Signed-off-by: Maxim Zhiburt <[email protected]>

zhiburt requested review from lunny, techknowlogick and adelowo October 18, 2020 22:22

zhiburt added 2 commits October 19, 2020 20:10

Use SendAsync+Flush instead of SendSync

5b5f8c5

Signed-off-by: Maxim Zhiburt <[email protected]>

Add timeout parameter to sendemail command

042c259

Signed-off-by: Maxim Zhiburt <[email protected]>

Fix spelling mistake

a25d8be

Signed-off-by: Maxim Zhiburt <[email protected]>

zhiburt requested review from 6543 and lafriks October 22, 2020 17:40

6543 reviewed Oct 22, 2020

View reviewed changes

cmd/admin.go Outdated Show resolved Hide resolved

6543 reviewed Oct 22, 2020

View reviewed changes

cmd/admin.go Outdated Show resolved Hide resolved

6543 approved these changes Oct 22, 2020

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 22, 2020

Update cmd/admin.go

b350eea

Co-authored-by: 6543 <[email protected]>

zhiburt added 2 commits October 24, 2020 00:02

Connect to a running Gitea instance

3d5b393

Merge branch 'sendmail_command' of github.com:zhiburt/gitea into send…

cee82ff

…mail_command

zhiburt added 2 commits October 24, 2020 00:06

Fix mispelling

6968eb9

Add copyright comment

af3cff6

lunny approved these changes Oct 24, 2020

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 24, 2020

lunny and others added 3 commits October 24, 2020 20:09

Merge branch 'master' into sendmail_command

b959bf1

Merge branch 'master' into sendmail_command

1b9f0ee

Merge branch 'master' into sendmail_command

490fa03

techknowlogick merged commit a1952af into go-gitea:master Oct 24, 2020

go-gitea locked and limited conversation to collaborators Dec 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sendmail command #13079

Sendmail command #13079

zhiburt commented Oct 8, 2020 •

edited

Loading

codecov-io commented Oct 8, 2020 •

edited

Loading

techknowlogick left a comment

zhiburt commented Oct 9, 2020

techknowlogick commented Oct 11, 2020

zhiburt commented Oct 12, 2020 •

edited

Loading

zeripath commented Oct 19, 2020

zhiburt commented Oct 19, 2020 •

edited

Loading

zhiburt commented Oct 22, 2020 •

edited

Loading

lunny commented Oct 23, 2020

zhiburt commented Oct 23, 2020 •

edited

Loading

techknowlogick commented Oct 24, 2020

Sendmail command #13079

Sendmail command #13079

Conversation

zhiburt commented Oct 8, 2020 • edited Loading

codecov-io commented Oct 8, 2020 • edited Loading

Codecov Report

techknowlogick left a comment

Choose a reason for hiding this comment

zhiburt commented Oct 9, 2020

techknowlogick commented Oct 11, 2020

zhiburt commented Oct 12, 2020 • edited Loading

zeripath commented Oct 19, 2020

zhiburt commented Oct 19, 2020 • edited Loading

zhiburt commented Oct 22, 2020 • edited Loading

lunny commented Oct 23, 2020

zhiburt commented Oct 23, 2020 • edited Loading

techknowlogick commented Oct 24, 2020

zhiburt commented Oct 8, 2020 •

edited

Loading

codecov-io commented Oct 8, 2020 •

edited

Loading

zhiburt commented Oct 12, 2020 •

edited

Loading

zhiburt commented Oct 19, 2020 •

edited

Loading

zhiburt commented Oct 22, 2020 •

edited

Loading

zhiburt commented Oct 23, 2020 •

edited

Loading