Skip to content

Conversation

@cweiske
Copy link
Contributor

@cweiske cweiske commented Jul 3, 2018

Located in Backend > Mails > Function check

Screenshot: http://fotos.cweiske.de/screenshots/2018/2018-07-03%20powermail%20delete%20all.png

@einpraegsam
Copy link
Collaborator

First of all: Thx for your work, I like your change and think we can merge such a feature. I also like the anchor for the email form.

But I would like to discuss with you:

  1. Isn't it a bit too complicated programed with a checkbox "keepall" and "reallydelete"? If this is just a security feature to prevent clicks by accident, I would simply add a JavaScript inline confirm(). This will keep the php code shorter then now I think.
  2. The action in backend is called "function check". But your new feature is not a function check. But I also have no idea where to place such a feature. If we would move this action to a different place, we could use an action method alone for this feature (at the moment one actions does two different things - what is not so nice in my eyes). Any ideas for a better place?

In addtion - please respect PSR-2 and a good coding style:

  • if else should be ifelse in Classes/Controller/ModuleController.php:190
  • Please compare in PHP only type secure === is always better then == (same line and 3 lines above)

@einpraegsam
Copy link
Collaborator

Any feedback? Otherwise I'm forced to close this PR.

@cweiske
Copy link
Contributor Author

cweiske commented Aug 6, 2018 via email

cweiske added a commit to mogic-le/powermail that referenced this pull request Mar 19, 2021
The "Mail list" page has a "cleanup" button that opens a small form
which allows one to delete all mails and answers on the current page.

Port of the old patch from 2018
 in2code-de#304
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants