[pmg-devel] [PATCH pmg-api/gui/docs v2] ruledb disclaimer improvements

[pmg-devel] [PATCH pmg-api/gui/docs v2] ruledb disclaimer improvements

[Dominik Csapak]

add 'position' and 'add-separator' options for disclaimers

maybe someone has a better idea for how i handle the database
updates

changes from v1:
* add docs patch
* rebase on master
* change 'top' parameter to 'position' (internally still 'top')
* change how we set default values, so that they don't end up in the
  database

pmg-api:

Dominik Csapak (2):
  fix #2606: ruledb disclaimer: add ability to set position
  fix #2430: ruledb disclaimer: make separator configurable

 src/PMG/RuleDB/Disclaimer.pm | 78 +++++++++++++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 5 deletions(-)

pmg-gui:

Dominik Csapak (2):
  disclaimer edit: add position comobobox
  disclaimer edit: add 'add-separator' checkbox

 js/Utils.js | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

pmg-docs:

Dominik Csapak (1):
  add disclaimer position/separator docs

 pmg-mail-filter.adoc | 5 +++++
 1 file changed, 5 insertions(+)

-- 
2.30.2

[pmg-devel] [PATCH pmg-api v2 1/2] fix #2606: ruledb disclaimer: add ability to set position

[Dominik Csapak]

there are usecases, where one wants to embed a disclaimer (or similar)
text or html at the beginning of the mail content, not at the end.

so introduce a 'position' parameter for the disclaimer that does exactly
that.

internally this will get saved as `top` only if it is not the default of '0'.
We save it in the `Attribut` table in that case.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* rename public property to 'position'
* change default handling in 'update' so that we only ever save the
  non-default value to the database
 src/PMG/RuleDB/Disclaimer.pm | 56 ++++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 5 deletions(-)

diff --git a/src/PMG/RuleDB/Disclaimer.pm b/src/PMG/RuleDB/Disclaimer.pm
index 24efa8a..421a0dd 100644
--- a/src/PMG/RuleDB/Disclaimer.pm
+++ b/src/PMG/RuleDB/Disclaimer.pm
@@ -49,7 +49,7 @@ If this e-mail or attached files contain information which do not relate to our
 _EOD_
 
 sub new {
-    my ($type, $value, $ogroup) = @_;
+    my ($type, $value, $ogroup, $top) = @_;
 
     my $class = ref($type) || $type;
 
@@ -58,6 +58,7 @@ sub new {
     my $self = $class->SUPER::new($class->otype(), $ogroup);
 
     $self->{value} = $value;
+    $self->{top} = $top;
 
     return $self;
 }
@@ -69,7 +70,20 @@ sub load_attr {
 
     defined($value) || die "undefined object attribute: ERROR";
 
-    my $obj = $class->new(decode('UTF-8', $value), $ogroup);
+    my $sth = $ruledb->{dbh}->prepare(
+	"SELECT * FROM Attribut WHERE Object_ID = ?");
+
+    $sth->execute($id);
+
+    my $top = 0;
+
+    while (my $ref = $sth->fetchrow_hashref()) {
+	$top = $ref->{value} if $ref->{name} eq 'top';
+    }
+
+    $sth->finish();
+
+    my $obj = $class->new(decode('UTF-8', $value), $ogroup, $top);
 
     $obj->{id} = $id;
 
@@ -96,6 +110,9 @@ sub save {
 	    "UPDATE Object SET Value = ? WHERE ID = ?",
 	    undef, $value, $self->{id});
 
+	$ruledb->{dbh}->do(
+	    "DELETE FROM Attribut WHERE Name = ? and Object_ID = ?",
+	    undef, 'top',  $self->{id});
     } else {
 	# insert
 
@@ -108,6 +125,12 @@ sub save {
 	$self->{id} = PMG::Utils::lastid($ruledb->{dbh}, 'object_id_seq');
     }
 
+    if (defined($self->{top})) {
+	$ruledb->{dbh}->do(
+	    "INSERT INTO Attribut (Value, Name, Object_ID) VALUES (?, ?, ?)",
+	    undef, $self->{top}, 'top',  $self->{id});
+    }
+
     return $self->{id};
 }
 
@@ -134,13 +157,19 @@ sub add_data {
 
     my $newfh = $body->open ("w") || return undef;
 
+    if ($self->{top}) {
+	$newfh->print($data);
+    }
+
     while (defined($_ = $fh->getline())) {
 	$newfh->print($_); # copy contents
     }
 
     $newfh->print("\n"); # add final \n
 
-    $newfh->print($data);
+    if (!$self->{top}) {
+	$newfh->print($data);
+    }
 
     $newfh->close || return undef;
 
@@ -197,8 +226,12 @@ sub execute {
 
     foreach my $ta (@$subgroups) {
 	my ($tg, $entity) = (@$ta[0], @$ta[1]);
-
-	my $html = "<br>--<br>" . PMG::Utils::subst_values ($self->{value}, $vars);
+	my $html;
+	if ($self->{top}) {
+	    $html = PMG::Utils::subst_values ($self->{value}, $vars) . "<br>--<br>";
+	} else {
+	    $html = "<br>--<br>" . PMG::Utils::subst_values ($self->{value}, $vars);
+	}
 
 	my $text = "";
 	my $parser = HTML::Parser->new(
@@ -232,6 +265,13 @@ sub properties {
 	    type => 'string',
 	    maxLength => 2048,
 	},
+	position => {
+	    description => "Put the disclaimer at the specified position.",
+	    type => 'string',
+	    enum => ['start', 'end'],
+	    optional => 1,
+	    default => 'end',
+	},
     };
 }
 
@@ -240,6 +280,7 @@ sub get {
 
     return {
 	disclaimer => $self->{value},
+	position => $self->{top} ? 'start' : 'end',
     };
 }
 
@@ -247,6 +288,11 @@ sub update {
     my ($self, $param) = @_;
 
     $self->{value} = $param->{disclaimer};
+    if (defined($param->{position}) && $param->{position} eq 'start') {
+	$self->{top} = 1;
+    } else {
+	delete $self->{top};
+    }
 }
 
 1;
-- 
2.30.2

[pmg-devel] [PATCH pmg-api v2 2/2] fix #2430: ruledb disclaimer: make separator configurable

[Dominik Csapak]

add a new 'add-separator' property (default true) that controls if the
'--' separator is added.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* change default handling so that only non default values land in the db
 src/PMG/RuleDB/Disclaimer.pm | 44 +++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/src/PMG/RuleDB/Disclaimer.pm b/src/PMG/RuleDB/Disclaimer.pm
index 421a0dd..574f62e 100644
--- a/src/PMG/RuleDB/Disclaimer.pm
+++ b/src/PMG/RuleDB/Disclaimer.pm
@@ -49,7 +49,7 @@ If this e-mail or attached files contain information which do not relate to our
 _EOD_
 
 sub new {
-    my ($type, $value, $ogroup, $top) = @_;
+    my ($type, $value, $ogroup, $top, $separator) = @_;
 
     my $class = ref($type) || $type;
 
@@ -59,6 +59,7 @@ sub new {
 
     $self->{value} = $value;
     $self->{top} = $top;
+    $self->{separator} = $separator;
 
     return $self;
 }
@@ -76,14 +77,16 @@ sub load_attr {
     $sth->execute($id);
 
     my $top = 0;
+    my $separator = 1;
 
     while (my $ref = $sth->fetchrow_hashref()) {
 	$top = $ref->{value} if $ref->{name} eq 'top';
+	$separator = $ref->{value} if $ref->{name} eq 'separator';
     }
 
     $sth->finish();
 
-    my $obj = $class->new(decode('UTF-8', $value), $ogroup, $top);
+    my $obj = $class->new(decode('UTF-8', $value), $ogroup, $top, $separator);
 
     $obj->{id} = $id;
 
@@ -110,9 +113,11 @@ sub save {
 	    "UPDATE Object SET Value = ? WHERE ID = ?",
 	    undef, $value, $self->{id});
 
-	$ruledb->{dbh}->do(
-	    "DELETE FROM Attribut WHERE Name = ? and Object_ID = ?",
-	    undef, 'top',  $self->{id});
+	for my $prop (qw(top separator)) {
+	    $ruledb->{dbh}->do(
+		"DELETE FROM Attribut WHERE Name = ? and Object_ID = ?",
+		undef, $prop,  $self->{id});
+	}
     } else {
 	# insert
 
@@ -125,10 +130,12 @@ sub save {
 	$self->{id} = PMG::Utils::lastid($ruledb->{dbh}, 'object_id_seq');
     }
 
-    if (defined($self->{top})) {
-	$ruledb->{dbh}->do(
-	    "INSERT INTO Attribut (Value, Name, Object_ID) VALUES (?, ?, ?)",
-	    undef, $self->{top}, 'top',  $self->{id});
+    for my $prop (qw(top separator)) {
+	if (defined($self->{$prop})) {
+	    $ruledb->{dbh}->do(
+		"INSERT INTO Attribut (Value, Name, Object_ID) VALUES (?, ?, ?)",
+		undef, $self->{$prop}, $prop,  $self->{id});
+	}
     }
 
     return $self->{id};
@@ -227,10 +234,11 @@ sub execute {
     foreach my $ta (@$subgroups) {
 	my ($tg, $entity) = (@$ta[0], @$ta[1]);
 	my $html;
+	my $separator = $self->{separator} ? '<br>--<br>' : '<br>';
 	if ($self->{top}) {
-	    $html = PMG::Utils::subst_values ($self->{value}, $vars) . "<br>--<br>";
+	    $html = PMG::Utils::subst_values ($self->{value}, $vars) . $separator;
 	} else {
-	    $html = "<br>--<br>" . PMG::Utils::subst_values ($self->{value}, $vars);
+	    $html = $separator . PMG::Utils::subst_values ($self->{value}, $vars);
 	}
 
 	my $text = "";
@@ -272,6 +280,13 @@ sub properties {
 	    optional => 1,
 	    default => 'end',
 	},
+	'add-separator' => {
+	    description => "If set to 1, adds a '--' separator between the disclaimer and the".
+		" content. Set to 0 to prevent that.",
+	    type => 'boolean',
+	    optional => 1,
+	    default => 1,
+	},
     };
 }
 
@@ -281,6 +296,7 @@ sub get {
     return {
 	disclaimer => $self->{value},
 	position => $self->{top} ? 'start' : 'end',
+	'add-separator' => $self->{separator} // 1,
     };
 }
 
@@ -293,6 +309,12 @@ sub update {
     } else {
 	delete $self->{top};
     }
+
+    if (defined($param->{'add-separator'}) && $param->{'add-separator'} == 0) {
+	$self->{separator} = 0;
+    } else {
+	delete $self->{separator};
+    }
 }
 
 1;
-- 
2.30.2

[pmg-devel] [PATCH pmg-gui v2 1/2] disclaimer edit: add position comobobox

[Dominik Csapak]

to enable setting the disclaimer position

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* change from checkbox to combobox, adapting to the backend changes
  improving clarity
 js/Utils.js | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/js/Utils.js b/js/Utils.js
index 7fa154e..974df1f 100644
--- a/js/Utils.js
+++ b/js/Utils.js
@@ -686,6 +686,17 @@ Ext.define('PMG.Utils', {
 		    growMax: 250,
 		    fieldLabel: gettext("Disclaimer"),
 		},
+		{
+		    xtype: 'proxmoxKVComboBox',
+		    name: 'position',
+		    fieldLabel: gettext("Position"),
+		    deleteEmpty: false,
+		    value: 'end',
+		    comboItems: [
+			['end', gettext('End')],
+			['start', gettext('Start')],
+		    ],
+		},
 	    ],
 	},
     },
-- 
2.30.2

[pmg-devel] [PATCH pmg-gui v2 2/2] disclaimer edit: add 'add-separator' checkbox

[Dominik Csapak]

for controlling if the separator should be included or not, default is
on (backend default).

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 js/Utils.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/js/Utils.js b/js/Utils.js
index 974df1f..4d591a7 100644
--- a/js/Utils.js
+++ b/js/Utils.js
@@ -697,6 +697,12 @@ Ext.define('PMG.Utils', {
 			['start', gettext('Start')],
 		    ],
 		},
+		{
+		    xtype: 'proxmoxcheckbox',
+		    name: 'add-separator',
+		    fieldLabel: gettext("Add Separator"),
+		    value: true,
+		},
 	    ],
 	},
     },
-- 
2.30.2

Re: [pmg-devel] [PATCH pmg-gui v2 2/2] disclaimer edit: add 'add-separator' checkbox

[Thomas Lamprecht]

Am 15/01/2024 um 12:12 schrieb Dominik Csapak:
> for controlling if the separator should be included or not, default is
> on (backend default).
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  js/Utils.js | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/js/Utils.js b/js/Utils.js
> index 974df1f..4d591a7 100644
> --- a/js/Utils.js
> +++ b/js/Utils.js
> @@ -697,6 +697,12 @@ Ext.define('PMG.Utils', {
>  			['start', gettext('Start')],
>  		    ],
>  		},
> +		{
> +		    xtype: 'proxmoxcheckbox',
> +		    name: 'add-separator',
> +		    fieldLabel: gettext("Add Separator"),
> +		    value: true,

this was missing setting the uncheckedValue to '0' explicitly, as otherwise
that value is undefined and won't get send at all if the checkbox is unchecked.

Mostly here an issue because most of the time our booleans represented by
checkboxes are opt-in, not opt-out (at least as shown to the user).

[pmg-devel] [PATCH pmg-docs v2 1/1] add disclaimer position/separator docs

[Dominik Csapak]

explain what the default is and that it can be configured.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v2
 pmg-mail-filter.adoc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/pmg-mail-filter.adoc b/pmg-mail-filter.adoc
index 3aafe4c..5cd752d 100644
--- a/pmg-mail-filter.adoc
+++ b/pmg-mail-filter.adoc
@@ -170,6 +170,11 @@ The disclaimer can contain HTML markup. It will be added to the first
 `text/html` and `text/plain` part of an email. A disclaimer only gets added if
 its text can be encoded in the mail's character encoding.
 
+By default it will be appended at the end of the selected part of the mail with
+`--` as a separator. The position (start or end of the selected part) and the
+existence of the separator can be configured with the `position` and
+`add-separator` options respectively.
+
 
 [[pmg_mailfilter_who]]
 'Who' objects
-- 
2.30.2

[pmg-devel] applied-series: [PATCH pmg-api/gui/docs v2] ruledb disclaimer improvements

[Thomas Lamprecht]

Am 15/01/2024 um 12:12 schrieb Dominik Csapak:
> add 'position' and 'add-separator' options for disclaimers
> 
> maybe someone has a better idea for how i handle the database
> updates
> 
> changes from v1:
> * add docs patch
> * rebase on master
> * change 'top' parameter to 'position' (internally still 'top')
> * change how we set default values, so that they don't end up in the
>   database
> 
> pmg-api:
> 
> Dominik Csapak (2):
>   fix #2606: ruledb disclaimer: add ability to set position
>   fix #2430: ruledb disclaimer: make separator configurable
> 
>  src/PMG/RuleDB/Disclaimer.pm | 78 +++++++++++++++++++++++++++++++++---
>  1 file changed, 73 insertions(+), 5 deletions(-)
> 
> pmg-gui:
> 
> Dominik Csapak (2):
>   disclaimer edit: add position comobobox
>   disclaimer edit: add 'add-separator' checkbox
> 
>  js/Utils.js | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> pmg-docs:
> 
> Dominik Csapak (1):
>   add disclaimer position/separator docs
> 
>  pmg-mail-filter.adoc | 5 +++++
>  1 file changed, 5 insertions(+)
> 


applied series, had to fix one issue in GUI patch 2/2 though (see reply
there), thanks!