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
Filter mails with Sieve #1822
Filter mails with Sieve #1822
Conversation
Looks great so far! Please coordinate with the others that wanted to work on #44. We should avoid conflicts and duplicate work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a migration in my stash. Looks very similar to your migration. We might need a sieve_enable
flag: $table->addColumn('sieve_enable', Type::BOOLEAN);
Signed-off-by: Pierre Gordon <pierregordon@protonmail.com>
Signed-off-by: Pierre Gordon <pierregordon@protonmail.com>
Signed-off-by: Pierre Gordon <pierregordon@protonmail.com>
Co-Authored-By: Daniel Kesselberg <mail@danielkesselberg.de> Signed-off-by: Pierre Gordon <pierregordon@protonmail.com>
Signed-off-by: Pierre Gordon <pierregordon@protonmail.com>
Signed-off-by: Pierre Gordon <pierregordon@protonmail.com>
Signed-off-by: Pierre Gordon <pierregordon@protonmail.com>
Signed-off-by: Pierre Gordon <pierregordon@protonmail.com>
8dc9f84
to
5500191
Compare
Co-Authored-By: Daniel Kesselberg <mail@danielkesselberg.de> Signed-off-by: Pierre Gordon <pierregordon@protonmail.com>
Signed-off-by: Pierre Gordon <pierregordon@protonmail.com>
…/feature/sieve-filters'
cb22a01
to
ea8101a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some early feedback :)
* @return ManageSieve | ||
* @throws ManageSieve\Exception | ||
*/ | ||
public function getSieveConnection() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getImapConnection
is using an outdated coding style and we're int process of refactoring our code to get rid of the method. Please see the ImapClientFactory
and create something similar for Sieve.
public function getSieveConnection() | ||
{ | ||
if ($this->sieveClient === null) { | ||
$host = $this->account->getSieveHost(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Handle accounts without Sieve settings
* @param int $accountId | ||
* @return JSONResponse | ||
* @throws AccountException | ||
* @throws \Horde\ManageSieve\Exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- TODO: Catch
* @param string $script | ||
* @param string $script_name | ||
* @return JSONResponse | ||
* @throws \Horde\ManageSieve\Exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- TODO: Catch
*/ | ||
private function getSieveService(int $accountId): SieveService | ||
{ | ||
$account = $this->accountService->find($this->currentUser->getUID(), $accountId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- TODO: handle account not found error
return $this->installRawScript($script); | ||
} | ||
|
||
return ['status' => 'error', 'message' => 'Not implemented']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
$this->installScript($script); | ||
return ['status' => 'success']; | ||
} else { | ||
return ['status' => 'error', 'message' => $script->getParseError()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
@@ -15,6 +15,8 @@ | |||
"test:watch": "mochapack -w --webpack-config webpack.test.js --require src/tests/setup.js \"src/tests/**/*.spec.js\"" | |||
}, | |||
"dependencies": { | |||
"@saeris/vue-spinners": "^1.0.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- TODO: use Nextcloud's
icon-loading
/icon-loading-small
instead
@@ -56,12 +58,15 @@ | |||
"vue-autosize": "^1.0.2", | |||
"vue-click-outside": "^1.0.7", | |||
"vue-infinite-scroll": "^2.0.2", | |||
"vue-m-message": "^2.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- TODO: use OC.Notification.showTemporary(...) instead
"vue-on-click-outside": "^1.0.3", | ||
"vue-router": "^3.1.3", | ||
"vue-scroll": "^2.1.12", | ||
"vue-select": "^3.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- TODO: use Multiselect from
nextcloud/vue
instead
@pierlon any updates? |
Currently there are some merge conflicts. @nextcloud/mail does anyone want to look into resolving them and/or taking this over? :) |
Whoever takes this over should first get in contact with the original authors so there is no conflict on who may eventually claim the bounty. |
@nextcloud/mail at which point will we just assume that this incentive is dead? We want to have this feature eventually but since I've been wrongly accused of taking away bounties from community people I'm not willing to take this over unless the original authors can clarify #1822 (comment). @pierlon @mmittelb would you have a minute to comment? |
Currently it's been 165d since the latest commit. |
Much apologies for not replying as quickly as possible here, but unfortunately I won't be able to continue where I left off. I hope the work done so far will serve as a basis and guide for the additional code to be written, and I do wish all the best for my successor. With that said, if I recall correctly I believe the "TODO" section in the description above is still accurate and those are still outstanding. I may have done some more work on the UI for adding and customizing filters that were not pushed; if I am able to find it I will update this PR with the changes. |
Sieve filters are a pretty important thing to me. I would try to move on here - even though I might need help for frontend things. |
I think so. The only other option would be pull requests to |
@dehnhardt please coordinate with the original authors regarding #1822 (comment) and how you want to handle the bounty. |
Ups, haven't seen the bounty. Ok, I will get in touch with pierlon. |
@pierlon : Dear Pierre, I want to try to finish this ManageSieve extension. |
You can try the email from the commit 6d7b0dc |
Hey @dehnhardt, I'd leave that up to you. I did put a lot of hard work into this quite some time ago, but I believe the person who is able to bring this across the finish line deserves as much respect and praise as I would have received if I did. |
@pierlon, then we'll see if I get further than you and if this is the case we'll toss a coin;-) |
Hi 👋, we are about to merge #4472. This isn't the full sieve feature but merely the basics with sieve credentials, provisioning and so on and "just" a basic UI, but this will be the foundation for any other enhancements. Therefore I'm closing this PR and the other one. Thanks a lot everyone who has helped move this forward. Even if this PR wasn't integrated it was a huge help for Daniel's implementation. ✌️ |
This PR is intended to fix #44.
Todo: