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

Add "bouncers reset", "bouncers del" and "bouncers [--status] review" commands to sympa.pl #1058

Closed
ldidry opened this issue Dec 17, 2020 · 26 comments · Fixed by #1098
Closed
Assignees
Milestone

Comments

@ldidry
Copy link
Contributor

ldidry commented Dec 17, 2020

Due to recent gmail problems, a lot of gmail.com subscribers are in error status in a lot of our lists.

Having a command line tool to cancel errors (and unsubscribe users in error status, since the option exists in the web UI) would be useful.

Note: I intend to code it myself next week, I’m creating this issue to think about it.

Expected Behavior

Being able to cancel errors on unsubscribe users in error status from command line with sympa.pl.

Current Behavior

Such option does not exist.

@ldidry ldidry self-assigned this Dec 17, 2020
@ikedas
Copy link
Member

ikedas commented Dec 17, 2020

My ideas:

  • The option like --show_status_of_members may also be useful.
  • Sympa's CLI prefers _ instead of - for separators of options (execpt leading ones). I don't know exact reason.

@racke
Copy link
Contributor

racke commented Dec 17, 2020

There is certainly no good reason for the underscores 🙄.

@ikedas
Copy link
Member

ikedas commented Dec 17, 2020

There is certainly no good reason for the underscores .

I agree. Perhaps this custom is inspired by Perl's syntax: Identifiers can contain _ but never -. Since someone began, followers adhered to keep consistency.

@ldidry
Copy link
Contributor Author

ldidry commented Dec 23, 2020

The option like --show_status_of_members may also be useful.

How would you like it? I can list the users without errors, then the users with errors, sorted alphabetically or sorted by the number of bounces, or list all users alphabetically with [BOUNCE(S): 13] before of after the address for those who have errors (13 is just an exemple, here).

@ikedas
Copy link
Member

ikedas commented Dec 24, 2020

@ldidry, flexible querying is nice idea, however we'd be better to start with minimum feature that dumps the bounce status of all subscribers in a list.

@ikedas
Copy link
Member

ikedas commented Jan 28, 2021

  • The word "subscribe"/"unsubscribe" has been used for the actions by the user to add/remove themselves. The word "add"/"del" has been used for the actions by administrators.
  • I feel the options are way too long.

@racke
Copy link
Contributor

racke commented Jan 28, 2021

Comment on option names: they are too long :-). The from-list part is superfluous. Also "reset" is more appropriate than "cancel" in my opinion. On top of that errors = bounces?

So my two cents:

  • reset_subscriber_bounces
  • unsubscribe_bounced_users
  • show_members_status

@ldidry
Copy link
Contributor Author

ldidry commented Jan 28, 2021

So my two cents:

  • reset_subscriber_bounces
  • unsubscribe_bounced_users
  • show_members_status

Ok for me. @ikedas, is that ok for you?

@ikedas
Copy link
Member

ikedas commented Jan 28, 2021

In my idea,

  • reset_member_bounce
  • del_bounced_member
  • show_member_status

subscriber, user -> member
subscribe -> del

I don't know whether plural or singular is better ("members" or "member").

@ldidry
Copy link
Contributor Author

ldidry commented Jan 28, 2021

I think plural is better, it won’t surprise admins that all bounced members will be deleted, and they will not be surprised either if there is only one. Singular would mean "specify the member to delete" to me.

@racke
Copy link
Contributor

racke commented Jan 28, 2021

Yes, I also think plural is better.

@ikedas
Copy link
Member

ikedas commented Jan 28, 2021

I think plural is better, it won’t surprise admins that all bounced members will be deleted, and they will not be surprised either if there is only one. Singular would mean "specify the member to delete" to me.

I see. I’m not familiar with distinction of plural/singular... Besides in my idea, FYI,

  • reset_member_bounces
  • del_bounced_members
  • show_members_status (perhaps show_member_statuses ?)

@ldidry
Copy link
Contributor Author

ldidry commented Jan 29, 2021

perhaps show_member_statuses

That would imply that the members have more than one status. Do they? Can they have bounce and suspend attributes set at the same time? (I have no idea what the suspend attribute is for, or what triggers it)

@ikedas
Copy link
Member

ikedas commented Jan 29, 2021

perhaps show_member_statuses

That would imply that the members have more than one status. Do they? Can they have bounce and suspend attributes set at the same time? (I have no idea what the suspend attribute is for, or what triggers it)

It's just an idea:
If we would integrate it with Sympa::Request framework by the future refactoring, the function to list the subscribers may be called "review", such as:

sympa.pl --review --list=list@domain

and to show them with status, either

sympa.pl --review --list=list@domain --verbose
sympa.pl --review --list=list@domain --status

@ldidry
Copy link
Contributor Author

ldidry commented Jan 29, 2021

I suggest dropping the --list:

sympa.pl --review list@domain [--status]

--verbose could be used in the future to print the informations of the web table: name, reception, sources, subscribed from, updated at.

@racke
Copy link
Contributor

racke commented Jan 29, 2021

Sounds like a good idea, so I suggest to create a separate issue for --review and remove shows_member_status from #1098.

@ldidry
Copy link
Contributor Author

ldidry commented Jan 29, 2021

I’m ok. I’ll wait for @ikedas’ opinion.

@ikedas
Copy link
Member

ikedas commented Jan 29, 2021

I'm ok too.

@ldidry
Copy link
Contributor Author

ldidry commented Jan 29, 2021

PR #1098 modified with --reset_members_bounces and --del_bounced_members options

@ikedas
Copy link
Member

ikedas commented Jan 27, 2022

Sorry to rehash this discussion, but things like the following are also possible.

sympa bouncers reset list@domain
sympa bouncers del list@domain
sympa review [--status] list@domain

@racke
Copy link
Contributor

racke commented Jan 27, 2022

Yeah that is definitely more concise 👍

@ldidry
Copy link
Contributor Author

ldidry commented Jan 27, 2022

Seems good to me. I will update the PR.

@ldidry
Copy link
Contributor Author

ldidry commented Feb 2, 2022

I updated my PR.

I followed your suggestion for

sympa bouncers reset list@domain
sympa bouncers del list@domain
sympa review [--status] list@domain

I just have a small problem: sympa review _list_ (without domain) works well, but sympa bouncers reset _list_ does not work (del does not work either), it does not add the domain to the list’s name.
Do you have an idea on how could I make it work?

@ikedas
Copy link
Member

ikedas commented Feb 3, 2022

I confirmed it works (with domain for a list in a virtual domain). How it didn't work on your side?

@ldidry
Copy link
Contributor Author

ldidry commented Feb 3, 2022

I thought it was because of my development VM, so I tried on my production server: still not working without the domain 🤔

# /home/sympa/bin/sympa bouncers reset foo
Invalid command 'foo'
# /home/sympa/bin/sympa bouncers reset foo@mydomain.org
No bounce errors to cancel.

The domain is defined in /etc/sympa/sympa.conf, though.

@ikedas
Copy link
Member

ikedas commented Feb 8, 2022

@ldidry , please check #1344.

@ikedas ikedas changed the title Add --cancel-errors-from-list and --unsubscribe-users-in-error-from-list to sympa.pl Add "bouncers reset", "bouncers del" and "bouncers [--status] review" commands to sympa.pl Feb 23, 2022
@ikedas ikedas closed this as completed in 1c56975 Mar 5, 2022
@ikedas ikedas added this to the 6.2.70 milestone Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants