[ENH] Added functionality to enable or disable Sieve Filters with toggle button #1104

amaninyumu1 · 2024-06-27T08:49:53Z

No description provided.

Shadow243

Please make sure all tests pass successfully and also remove composer.lock it will be supported by this #1093 because we have some changes that must be applied in sievefilters/modules.php before.

amaninyumu1 · 2024-09-05T14:29:48Z

hello @Shadow243 . I deleted the composer.lock file as you requested in your previous comment and all the tests passed successfully. please review

marclaporte · 2024-09-12T03:53:59Z

Sorry, but there is another conflict.

amaninyumu1 · 2024-09-12T18:23:32Z

hello @marclaporte . conflict has been resolved

marclaporte · 2024-09-13T22:43:15Z

modules/sievefilters/modules.php

@@ -34,7 +34,7 @@ public function process() {
            $this->out('conditions', json_encode(base64_decode($base64_obj)));
            $base64_obj = str_replace("# ", "", preg_split('#\r?\n#', $script, 0)[2]);
            $this->out('actions', json_encode(base64_decode($base64_obj)));
-            if (mb_strstr($script, 'allof')) {
+            if (strstr($script, 'allof')) {


Is this intentional?

I am wondering because of #1224

We should keep multibyte functions in 99% of the places. There was only one exception when the size of a string was specified in bytes in the IMAP protocol (literals).

marclaporte · 2024-09-13T22:43:28Z

modules/sievefilters/modules.php

@@ -223,7 +263,7 @@ public function process() {
            $blocked_wildcard = '@'.$domain;
            $new_blocked_list = [];
            foreach ($blocked_list as $idx => $blocked_sender) {
-                if (!mb_strstr($blocked_sender, $blocked_wildcard)) {
+                if (!strstr($blocked_sender, $blocked_wildcard)) {


Is this intentional?

I am wondering because of #1224

no it was not intentional. I understand, I correct

marclaporte · 2024-09-13T22:43:41Z

modules/sievefilters/modules.php

@@ -635,7 +675,7 @@ public function process() {
        $script_name = generate_filter_name($this->request->post['sieve_filter_name'], $priority);
        $conditions = json_decode($this->request->post['conditions_json']);
        $actions = json_decode($this->request->post['actions_json']);
-        $test_type = mb_strtolower($this->request->post['filter_test_type']);
+        $test_type = strtolower($this->request->post['filter_test_type']);


Is this intentional?

I am wondering because of #1224

no it was not intentional. I understand, I correct

kroky · 2024-09-16T05:55:01Z

composer.json

@@ -46,7 +46,7 @@
        "ext-openssl": "*",
        "ext-session": "*",
        "ezyang/htmlpurifier": "^4.17",
-        "henrique-borba/php-sieve-manager": "^1.0",
+        "henrique-borba/php-sieve-manager": "~1.0.4",


We don't want such a hard constraint to this lib. It will prevent upgrades of the lib for non-major versions as 1.1.0.

modules/sievefilters/site.js

kroky · 2024-09-16T06:02:02Z

modules/sievefilters/modules.php

                );
            }
            if ($action->action == 'flag') {
                $custom_condition->addAction(
-                    new \PhpSieveManager\Filters\Actions\FlagFilterAction(['flags' => [$action->value]])
+                    new \PhpSieveManager\Filters\Actions\FlagFilterAction([$action->value])


Why do you change all these FilterAction calls to constructors? I believe php-sieve-manager still expects the params in an associative array, doesn't it?

@kroky means that php-sieve-manager expects to receive associative arrays. with the changes you have applied, it will break sieve.

amaninyumu1 · 2024-09-18T16:27:23Z

Hello @kroky
I added echo json_encode([...]); to send a json response to ajax

modules/sievefilters/modules.php

…' of https://github.com/amaninyumu1/cypht into add-a-toggle-top-activate-deactivate-a-Cypht-Sieve-rule

Shadow243

It seems to me that you did a rebase which removed some important behavior. Can you review this PR carefully?

Shadow243 · 2024-09-05T14:40:04Z

composer.json

@@ -46,7 +46,7 @@
        "ext-openssl": "*",
        "ext-session": "*",
        "ezyang/htmlpurifier": "^4.17",
-        "henrique-borba/php-sieve-manager": "^1.0",
+        "henrique-borba/php-sieve-manager": "~1.0.4",


You don't need this too

Shadow243 · 2024-09-27T13:31:29Z

modules/sievefilters/modules.php

                );
            }
            if ($action->action == 'flag') {
                $custom_condition->addAction(
-                    new \PhpSieveManager\Filters\Actions\FlagFilterAction(['flags' => [$action->value]])
+                    new \PhpSieveManager\Filters\Actions\FlagFilterAction([$action->value])


@kroky means that php-sieve-manager expects to receive associative arrays. with the changes you have applied, it will break sieve.

Shadow243 · 2024-09-27T13:40:33Z

modules/sievefilters/site.js

@@ -130,13 +130,6 @@ var hm_sieve_possible_actions = function() {
            type: 'string',
            extra_field: false
        },
-        {


we are using this filter at https://github.com/cypht-org/cypht/blob/master/modules/sievefilters/modules.php#L915

I mean I don't understand why you are deleting this here.

From what I understand, the value $action->value passed into the addAction method is not explicitly an associative array in the current implementation. Could you please explain to me how this change might "break" sieve's behavior? It would help me better understand the issue and ensure that the implementation meets the expectations of PhpSieveManager. thanks

Shadow243 · 2024-09-27T13:48:09Z

modules/sievefilters/site.js

@@ -539,12 +532,6 @@ $(function () {
            $('#stop_filtering').prop('checked', false);
            current_account = $(this).attr('account');
            edit_filter_modal.open();
-
-            // Reset the form fields when opening the modal
-            $(".modal_sieve_filter_name").val('');


By deleting this code, the form starts to keep the previous data again because it is this code which resets when we click on add again.

Shadow243 · 2024-09-27T13:51:56Z

modules/sievefilters/modules.php

@@ -1179,9 +1216,6 @@ protected function output() {
        $res .= get_script_modal_content();
        $res .= '<div class="p-3">';
        foreach($mailboxes as $idx => $mailbox) {
-            if (empty($mailbox['sieve_config_host'])) {


This condition remains important, isn't it?

amaninyumu1 force-pushed the add-a-toggle-top-activate-deactivate-a-Cypht-Sieve-rule branch from a8bd54f to b330c2b Compare June 27, 2024 08:57

josaphatim force-pushed the add-a-toggle-top-activate-deactivate-a-Cypht-Sieve-rule branch 2 times, most recently from fb9dc09 to 0bd8033 Compare July 2, 2024 15:02

amaninyumu1 marked this pull request as ready for review July 4, 2024 20:04

marclaporte requested a review from Shadow243 July 11, 2024 18:17

Shadow243 requested changes Jul 25, 2024

View reviewed changes

amaninyumu1 force-pushed the add-a-toggle-top-activate-deactivate-a-Cypht-Sieve-rule branch 2 times, most recently from aedb2a1 to fd4fdf2 Compare August 18, 2024 16:02

marclaporte requested a review from Shadow243 August 25, 2024 09:23

amaninyumu1 force-pushed the add-a-toggle-top-activate-deactivate-a-Cypht-Sieve-rule branch from fd4fdf2 to 37133d4 Compare September 12, 2024 18:22

marclaporte requested a review from kroky September 13, 2024 11:44

marclaporte reviewed Sep 13, 2024

View reviewed changes

kroky reviewed Sep 16, 2024

View reviewed changes

modules/sievefilters/site.js Outdated Show resolved Hide resolved

kroky reviewed Sep 16, 2024

View reviewed changes

amaninyumu1 force-pushed the add-a-toggle-top-activate-deactivate-a-Cypht-Sieve-rule branch 3 times, most recently from 768e378 to 5929ff7 Compare September 18, 2024 16:24

kroky reviewed Sep 19, 2024

View reviewed changes

modules/sievefilters/modules.php Outdated Show resolved Hide resolved

amaninyumu1 force-pushed the add-a-toggle-top-activate-deactivate-a-Cypht-Sieve-rule branch 4 times, most recently from 86542ca to 6ccac96 Compare September 20, 2024 08:19

Merge branch 'add-a-toggle-top-activate-deactivate-a-Cypht-Sieve-rule…

6ccac96

…' of https://github.com/amaninyumu1/cypht into add-a-toggle-top-activate-deactivate-a-Cypht-Sieve-rule

Shadow243 requested changes Sep 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENH] Added functionality to enable or disable Sieve Filters with toggle button #1104

[ENH] Added functionality to enable or disable Sieve Filters with toggle button #1104

amaninyumu1 commented Jun 27, 2024

Shadow243 left a comment

amaninyumu1 commented Sep 5, 2024

marclaporte commented Sep 12, 2024

amaninyumu1 commented Sep 12, 2024

marclaporte Sep 13, 2024

kroky Sep 16, 2024

marclaporte Sep 13, 2024

amaninyumu1 Sep 16, 2024

marclaporte Sep 13, 2024

amaninyumu1 Sep 16, 2024

kroky Sep 16, 2024

kroky Sep 16, 2024

amaninyumu1 Sep 16, 2024

Shadow243 Sep 27, 2024

amaninyumu1 commented Sep 18, 2024

Shadow243 left a comment

Shadow243 Sep 5, 2024

Shadow243 Sep 27, 2024

Shadow243 Sep 27, 2024

amaninyumu1 Sep 29, 2024

Shadow243 Sep 27, 2024

Shadow243 Sep 27, 2024

amaninyumu1 Sep 29, 2024

[ENH] Added functionality to enable or disable Sieve Filters with toggle button #1104

Are you sure you want to change the base?

[ENH] Added functionality to enable or disable Sieve Filters with toggle button #1104

Conversation

amaninyumu1 commented Jun 27, 2024

Shadow243 left a comment

Choose a reason for hiding this comment

amaninyumu1 commented Sep 5, 2024

marclaporte commented Sep 12, 2024

amaninyumu1 commented Sep 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amaninyumu1 commented Sep 18, 2024

Shadow243 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment