Handling of (un)subscribe mails for already (un)subscribed users
Hi,
I noticed that a mail to -subscribe for an already subscribed user doesn't get any response (about line 225 in mailman/src/mailman/app/subscriptions.py). Same but different for unsubscribe for users not on the list.
I'd need these kind of mails however in hope that users complain less.
Would that be generally interessting, and if so, any special requirements for submitting patches, or should I just do that locally?
Regards, Andi
On 12/10/20 11:51 AM, Andreas Barth wrote:
Hi,
I noticed that a mail to -subscribe for an already subscribed user doesn't get any response (about line 225 in mailman/src/mailman/app/subscriptions.py). Same but different for unsubscribe for users not on the list.
I tested and I see this, so there is a bug, but I don't know what the issue is. The code you point to should raise AlreadySubscribedError and <https://gitlab.com/mailman/mailman/-/blob/master/src/mailman/commands/eml_membership.py#L110> should catch it and report, but this doesn't seem to be happening..
Also, <https://gitlab.com/mailman/mailman/-/blob/master/src/mailman/commands/eml_membership.py#L210> should report an attempt to unsubscribe a non-member, but the issue here is <https://gitlab.com/mailman/mailman/-/blob/master/src/mailman/runners/command.py#L70> which supresses sending the response because it is redundant if the user gets a 'you have been removed' email, but in this case there isn't one.
I'd need these kind of mails however in hope that users complain less.
Would that be generally interessting, and if so, any special requirements for submitting patches, or should I just do that locally?
These things should be fixed, but it's more involved than just a patch to fix the error. For all but trivial fixes, the workflow should be something along the lines of the following.
There should be an issue on GitLab for the appropriate project; in this case at <https://gitlab.com/mailman/mailman/-/issues>.
Then to develop a fix, there needs to be a fix, a test for the fixed code and a note in the NEWS.rst file about the fix. I generally prefer to start with a local clone of the GitLab branch and first create a test for the desired behavior. This test should fail. Then I create the fix, and now the test should pass. Finally, I create the NEWS item and commit those changes and create a merge request.
-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan
- Mark Sapiro (mark@msapiro.net) [201210 22:51]:
On 12/10/20 11:51 AM, Andreas Barth wrote:
Hi,
I noticed that a mail to -subscribe for an already subscribed user doesn't get any response (about line 225 in mailman/src/mailman/app/subscriptions.py). Same but different for unsubscribe for users not on the list.
I tested and I see this, so there is a bug, but I don't know what the issue is.
Ok, if you consider this a bug it makes sense to fix it upstream and not just locally.
The code you point to should raise AlreadySubscribedError and <https://gitlab.com/mailman/mailman/-/blob/master/src/mailman/commands/eml_membership.py#L110> should catch it and report, but this doesn't seem to be happening..
Actually I don't see where it should send the mail in the code you refered to, but I'm pretty new to mm3-code and the involved coding guidelines.
I would tend to add something along msg = UserNotification( self.mlist.owner_address, self.mlist.owner_address, subject, text, self.mlist.preferred_language) msg.send(self.mlist) (with appropriate subject, ...) before https://gitlab.com/mailman/mailman/-/blob/master/src/mailman/app/subscriptio... but of course that could be the wrong approach (but as subscriptions.py is sending out mail at other places, that's my first idea).
These things should be fixed, but it's more involved than just a patch to fix the error. For all but trivial fixes, the workflow should be something along the lines of the following. [...]
Thanks, I'll try to have a look into that.
Regards, Andi
On 12/10/20 2:04 PM, Andreas Barth wrote:
- Mark Sapiro (mark@msapiro.net) [201210 22:51]:
On 12/10/20 11:51 AM, Andreas Barth wrote:
Hi,
I noticed that a mail to -subscribe for an already subscribed user doesn't get any response (about line 225 in mailman/src/mailman/app/subscriptions.py). Same but different for unsubscribe for users not on the list.
I tested and I see this, so there is a bug, but I don't know what the issue is.
Ok, if you consider this a bug it makes sense to fix it upstream and not just locally.
The code you point to should raise AlreadySubscribedError and <https://gitlab.com/mailman/mailman/-/blob/master/src/mailman/commands/eml_membership.py#L110> should catch it and report, but this doesn't seem to be happening..
Actually I don't see where it should send the mail in the code you refered to, but I'm pretty new to mm3-code and the involved coding guidelines.
Ther code pointed to adds the error to results and returns
ContinueProcessing.yes. The return is to mailman/runners/command.py
which is ultimately responsible for returning results to the user in a
results of your email commands message.
The issue is the code at <https://gitlab.com/mailman/mailman/-/blob/master/src/mailman/app/subscriptions.py#L224> appears to not be executed as in my test at least, the workflow continued.
I would tend to add something along msg = UserNotification( self.mlist.owner_address, self.mlist.owner_address, subject, text, self.mlist.preferred_language) msg.send(self.mlist) (with appropriate subject, ...) before https://gitlab.com/mailman/mailman/-/blob/master/src/mailman/app/subscriptio... but of course that could be the wrong approach (but as subscriptions.py is sending out mail at other places, that's my first idea).
Aside from the fact that this wouldn't work if that code isn't executed, sending that message at that point would not be appropriate if the subscription request originated from something other than an email command. I.e., if it's a web subscribe, the error should be reported in the web response, not in an email to the user who may not even be the one originating the request.
-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan
Hi Mark,
thank you for your help.
- Mark Sapiro (mark@msapiro.net) [201210 23:26]:
The issue is the code at <https://gitlab.com/mailman/mailman/-/blob/master/src/mailman/app/subscriptions.py#L224> appears to not be executed as in my test at least, the workflow continued.
Interessting. Here it ended with (current git version)
==> /var/log/mailman3/mailman.log <== Dec 13 23:27:04 2020 (15015) deque: Traceback (most recent call last): File "/srv/prod.mailman/lib/python3/dist-packages/mailman/app/workflow.py", line 69, in __next__ return step() File "/srv/prod.mailman/lib/python3/dist-packages/mailman/app/subscriptions.py", line 227, in _step_sanity_checks MemberRole.member) mailman.interfaces.member.AlreadySubscribedError: t3@$domain is already a MemberRole.member of mailing list t1@$domain
This was a "live" run of mailman (however on my testing box). And, however, still no mail. Now I need to think how to continue.
Regards, Andi
- Andreas Barth (aba@ayous.org) [201213 23:44]:
==> /var/log/mailman3/mailman.log <== Dec 13 23:27:04 2020 (15015) deque: Traceback (most recent call last): File "/srv/prod.mailman/lib/python3/dist-packages/mailman/app/workflow.py", line 69, in __next__ return step() File "/srv/prod.mailman/lib/python3/dist-packages/mailman/app/subscriptions.py", line 227, in _step_sanity_checks MemberRole.member) mailman.interfaces.member.AlreadySubscribedError: t3@$domain is already a MemberRole.member of mailing list t1@$domain
This was a "live" run of mailman (however on my testing box). And, however, still no mail. Now I need to think how to continue.
Further debugging showed that finder.send_response is not True around line 217 in src/mailman/runners/command.py, so the code breaks there.
Turns out that the cause for that behaviour is that I requested the subscribe via the -subscribe-subadress.
If I send the subscribe via a mail to -request with subscribe within the mail, it works as it should and I get a mail back with: t3@$domain is already a MemberRole.member of mailing list t1@$domain
Regards, Andi
On Tue, Dec 15, 2020, at 3:36 AM, Andreas Barth wrote:
- Andreas Barth (aba@ayous.org) [201213 23:44]:
==> /var/log/mailman3/mailman.log <== Dec 13 23:27:04 2020 (15015) deque: Traceback (most recent call last): File "/srv/prod.mailman/lib/python3/dist-packages/mailman/app/workflow.py", line 69, in __next__ return step() File "/srv/prod.mailman/lib/python3/dist-packages/mailman/app/subscriptions.py", line 227, in _step_sanity_checks MemberRole.member) mailman.interfaces.member.AlreadySubscribedError: t3@$domain is already a MemberRole.member of mailing list t1@$domain
This was a "live" run of mailman (however on my testing box). And, however, still no mail. Now I need to think how to continue.
Further debugging showed that finder.send_response is not True around line 217 in src/mailman/runners/command.py, so the code breaks there.
Turns out that the cause for that behaviour is that I requested the subscribe via the -subscribe-subadress.
If I send the subscribe via a mail to -request with subscribe within the mail, it works as it should and I get a mail back with: t3@$domain is already a MemberRole.member of mailing list t1@$domain
I faintly remember that this behavior was made intentionally to avoid duplicate messages sent for emails sent to -join, -leave and -confirm addresses since they already have some automatic response upon successful completion.
We probably should send the response if the request wasn't completed successfully, like subscribe failed for -join, leave failed for -leave and so on and so forth.
This is the line in code which avoids sending the response for emails sent to sub-addresses like -join, -leave and -confirm: https://gitlab.com/mailman/mailman/-/blob/master/src/mailman/runners/command...
We need some notion of success & failure here to figure out when to send response, currently there is only one response from the the command_finder and that is whether to continue processing or not.
Regards, Andi
Mailman-users mailing list -- mailman-users@mailman3.org To unsubscribe send an email to mailman-users-leave@mailman3.org https://lists.mailman3.org/mailman3/lists/mailman-users.mailman3.org/
-- thanks, Abhilash Raj (maxking)
- Abhilash Raj (maxking@asynchronous.in) [201215 06:58]:
I faintly remember that this behavior was made intentionally to avoid duplicate messages sent for emails sent to -join, -leave and -confirm addresses since they already have some automatic response upon successful completion.
Looking at the code I'm pretty sure that was on purpose. :)
We probably should send the response if the request wasn't completed successfully, like subscribe failed for -join, leave failed for -leave and so on and so forth.
I agree. However, not yet sure how, but that could be the next step.
For tracking the issue I added https://gitlab.com/mailman/mailman/-/issues/804
Regards, Andi
participants (3)
-
Abhilash Raj -
Andreas Barth -
Mark Sapiro