[pbs-devel] [PATCH proxmox-backup] notifications: adapt to proxmox-apt making old_version optional

[Lukas Wagner]

On Thu Nov 13, 2025 at 1:16 PM CET, Thomas Lamprecht wrote:
> Am 13.11.25 um 12:40 schrieb Shannon Sterz:
>> Signed-off-by: Shannon Sterz <s.sterz at proxmox.com>
>> ---
>>  src/server/notifications/template_data.rs | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/src/server/notifications/template_data.rs b/src/server/notifications/template_data.rs
>> index 0dcc5ed3..9adc4699 100644
>> --- a/src/server/notifications/template_data.rs
>> +++ b/src/server/notifications/template_data.rs
>> @@ -177,7 +177,7 @@ impl PackageUpdatesTemplateData {
>>                  .map(|info| UpgradablePackage {
>>                      package_name: info.package.clone(),
>>                      available_version: info.version.clone(),
>> -                    installed_version: info.old_version.clone(),
>> +                    installed_version: info.old_version.clone().unwrap_or_default(),
>>                  })
>>                  .collect(),
>>          }
>
> What about adapting installed_version to reality and make it also an
> option? I mean, depends a bit on how handlebars will handle such things
> then, and also how willing we're adopting light breakage if it fixes
> wrong historic assumptions.
>
> Our shipped template for this default/package-updates-body.txt.hbs should
> be updated in any case though, maybe special case the no installed_version,
> be that None or "", and add a NEW or something like that there.

Yes, I think adapting the template would definitely makes sense. I
vaguely remember seeing "null" entries there at some point but didn't
investigate back then; I guess these must have been new packages.

With regards to adapting the `installed_version` member, once the
template is adapted, it probably does not make difference for the
template, since both `null` and `""` would evaluate to 'false' in the
if-statement which would need to be added. But for correctness, changing
it to be an Option<String> definitely makes sense to me.