[Brainstorming] What XAR file type for Mail Template pages?

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[Brainstorming] What XAR file type for Mail Template pages?

vmassol
Administrator
Hi,

It seems we forgot to handle mail template pages. For example XWiki.ResetPasswordMailContent

We need to decide the type: demo, default, etc.

WDYT about demo (i.e. as soon as the user starts modifying it, we don’t upgrade it anymore)?

Thanks
-Vincent

Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] What XAR file type for Mail Template pages?

Thomas Mortagne
Administrator
On Mon, May 7, 2018 at 4:33 PM, Vincent Massol <[hidden email]> wrote:

> Hi,
>
> It seems we forgot to handle mail template pages. For example XWiki.ResetPasswordMailContent
>
> We need to decide the type: demo, default, etc.
>
> WDYT about demo (i.e. as soon as the user starts modifying it, we don’t upgrade it anymore)?
>
> Thanks
> -Vincent
>

All types with allowed edit prevent upgrade.

I think a more important question is: is it OK to delete it ?

Seems to me delete is not OK in this context. Unless it's possible to
change the mail template used for password reset ?

--
Thomas Mortagne
Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] What XAR file type for Mail Template pages?

vmassol
Administrator


> On 7 May 2018, at 16:48, Thomas Mortagne <[hidden email]> wrote:
>
> On Mon, May 7, 2018 at 4:33 PM, Vincent Massol <[hidden email]> wrote:
>> Hi,
>>
>> It seems we forgot to handle mail template pages. For example XWiki.ResetPasswordMailContent
>>
>> We need to decide the type: demo, default, etc.
>>
>> WDYT about demo (i.e. as soon as the user starts modifying it, we don’t upgrade it anymore)?
>>
>> Thanks
>> -Vincent
>>
>
> All types with allowed edit prevent upgrade.

I’m not sure we need more than 1 such type. See other mail thread.

> I think a more important question is: is it OK to delete it ?

We could. See below

>
> Seems to me delete is not OK in this context. Unless it's possible to
> change the mail template used for password reset ?

Re delete, I think there’s another thread discussing it, no? I don’t remember the discussion too well and don’t master all the details but AFAIR my preference was to not prevent deletion in general (I’m worried about unplanned use cases requiring a delete, like renaming the page to another place to save it, and then import some XAR containing the new mail template).

IMO all pages should be deletable without endangering the system. In this case we could imagine:
* if the template is missing then the password reset page would mention it with the ability to create a default mail template
* and/or report a mail error in the admin UI when sending the email (since the template doesn’t exist). This means that the template factory for emails should check the existence of the page. This should be handled here: https://github.com/xwiki/xwiki-platform/blob/6e281a093d3751666fdcd3fb3a69cb638cca9b59/xwiki-platform-core/xwiki-platform-mail/xwiki-platform-mail-send/xwiki-platform-mail-send-default/src/main/java/org/xwiki/mail/internal/factory/template/DefaultMailTemplateManager.java#L143 AFAICS it will currently report a NPE….

Have we decided what we do about deletes in general?

Thanks
-Vincent

> --
> Thomas Mortagne

Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] What XAR file type for Mail Template pages?

Ecaterina Moraru (Valica)
In reply to this post by Thomas Mortagne
On Mon, May 7, 2018 at 5:48 PM, Thomas Mortagne <[hidden email]>
wrote:

> On Mon, May 7, 2018 at 4:33 PM, Vincent Massol <[hidden email]> wrote:
> > Hi,
> >
> > It seems we forgot to handle mail template pages. For example
> XWiki.ResetPasswordMailContent
> >
> > We need to decide the type: demo, default, etc.
> >
> > WDYT about demo (i.e. as soon as the user starts modifying it, we don’t
> upgrade it anymore)?
> >
> > Thanks
> > -Vincent
> >
>
> All types with allowed edit prevent upgrade.
>
> I think a more important question is: is it OK to delete it ?
>
> Seems to me delete is not OK in this context. Unless it's possible to
> change the mail template used for password reset ?
>

If someone delete it, can't we auto-generate it blank from code?

I also see the templates (validation, confirmation, invitation, etc.) as
demo. They should be modified by the user. We just need to make sure they
don't contain too much code / velocity variables definitions that could
benefit from an upgrade.


>
> --
> Thomas Mortagne
>
Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] What XAR file type for Mail Template pages?

Thomas Mortagne
Administrator
In reply to this post by vmassol
On Mon, May 7, 2018 at 5:02 PM, Vincent Massol <[hidden email]> wrote:

>
>
>> On 7 May 2018, at 16:48, Thomas Mortagne <[hidden email]> wrote:
>>
>> On Mon, May 7, 2018 at 4:33 PM, Vincent Massol <[hidden email]> wrote:
>>> Hi,
>>>
>>> It seems we forgot to handle mail template pages. For example XWiki.ResetPasswordMailContent
>>>
>>> We need to decide the type: demo, default, etc.
>>>
>>> WDYT about demo (i.e. as soon as the user starts modifying it, we don’t upgrade it anymore)?
>>>
>>> Thanks
>>> -Vincent
>>>
>>
>> All types with allowed edit prevent upgrade.
>
> I’m not sure we need more than 1 such type. See other mail thread.
>
>> I think a more important question is: is it OK to delete it ?
>
> We could. See below
>
>>
>> Seems to me delete is not OK in this context. Unless it's possible to
>> change the mail template used for password reset ?
>
> Re delete, I think there’s another thread discussing it, no? I don’t remember the discussion too well and don’t master all the details but AFAIR my preference was to not prevent deletion in general (I’m worried about unplanned use cases requiring a delete, like renaming the page to another place to save it, and then import some XAR containing the new mail template).
>
> IMO all pages should be deletable without endangering the system. In this case we could imagine:
> * if the template is missing then the password reset page would mention it with the ability to create a default mail template
> * and/or report a mail error in the admin UI when sending the email (since the template doesn’t exist). This means that the template factory for emails should check the existence of the page. This should be handled here: https://github.com/xwiki/xwiki-platform/blob/6e281a093d3751666fdcd3fb3a69cb638cca9b59/xwiki-platform-core/xwiki-platform-mail/xwiki-platform-mail-send/xwiki-platform-mail-send-default/src/main/java/org/xwiki/mail/internal/factory/template/DefaultMailTemplateManager.java#L143 AFAICS it will currently report a NPE….

As you said, deleting that page would break reset password feature and
since I don't plan to rewrite it right now it means delete should be
protected IMO. If someone improve this feature later then the type can
be changed to "demo".

>
> Have we decided what we do about deletes in general?

There hasn't been such discussion. I don't even understand what this
mean, it's obvious to me that deleting some pages don't break anything
while for others you are going to create a huge mess.

>
> Thanks
> -Vincent
>
>> --
>> Thomas Mortagne
>



--
Thomas Mortagne
Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] What XAR file type for Mail Template pages?

vmassol
Administrator


> On 7 May 2018, at 18:18, Thomas Mortagne <[hidden email]> wrote:
>
> On Mon, May 7, 2018 at 5:02 PM, Vincent Massol <[hidden email]> wrote:
>>
>>
>>> On 7 May 2018, at 16:48, Thomas Mortagne <[hidden email]> wrote:
>>>
>>> On Mon, May 7, 2018 at 4:33 PM, Vincent Massol <[hidden email]> wrote:
>>>> Hi,
>>>>
>>>> It seems we forgot to handle mail template pages. For example XWiki.ResetPasswordMailContent
>>>>
>>>> We need to decide the type: demo, default, etc.
>>>>
>>>> WDYT about demo (i.e. as soon as the user starts modifying it, we don’t upgrade it anymore)?
>>>>
>>>> Thanks
>>>> -Vincent
>>>>
>>>
>>> All types with allowed edit prevent upgrade.
>>
>> I’m not sure we need more than 1 such type. See other mail thread.
>>
>>> I think a more important question is: is it OK to delete it ?
>>
>> We could. See below
>>
>>>
>>> Seems to me delete is not OK in this context. Unless it's possible to
>>> change the mail template used for password reset ?
>>
>> Re delete, I think there’s another thread discussing it, no? I don’t remember the discussion too well and don’t master all the details but AFAIR my preference was to not prevent deletion in general (I’m worried about unplanned use cases requiring a delete, like renaming the page to another place to save it, and then import some XAR containing the new mail template).
>>
>> IMO all pages should be deletable without endangering the system. In this case we could imagine:
>> * if the template is missing then the password reset page would mention it with the ability to create a default mail template
>> * and/or report a mail error in the admin UI when sending the email (since the template doesn’t exist). This means that the template factory for emails should check the existence of the page. This should be handled here: https://github.com/xwiki/xwiki-platform/blob/6e281a093d3751666fdcd3fb3a69cb638cca9b59/xwiki-platform-core/xwiki-platform-mail/xwiki-platform-mail-send/xwiki-platform-mail-send-default/src/main/java/org/xwiki/mail/internal/factory/template/DefaultMailTemplateManager.java#L143 AFAICS it will currently report a NPE….
>
> As you said, deleting that page would break reset password feature and
> since I don't plan to rewrite it right now it means delete should be
> protected IMO. If someone improve this feature later then the type can
> be changed to "demo".
>
>>
>> Have we decided what we do about deletes in general?
>
> There hasn't been such discussion.

I’m referring to http://markmail.org/message/kjtyzvjp5zzh4gyf (and I’m sure I saw another discussion about that but cannot find it ATM).

I still don’t understand why we’re mixing upgradability with deletability (and trying to find names that represent both). Aren’t they 2 different topics?

Thanks
-Vincent

> I don't even understand what this
> mean, it's obvious to me that deleting some pages don't break anything
> while for others you are going to create a huge mess.
>
>>
>> Thanks
>> -Vincent
>>
>>> --
>>> Thomas Mortagne
>>
>
>
>
> --
> Thomas Mortagne

Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] What XAR file type for Mail Template pages?

Eduard Moraru
Hi,

IMO, it's just like any other page: it depends on the application it is
part of and we can't treat them all the same.

XWiki.ResetPasswordMailContent should be "default". I see no particular
reason to customize the reset password email, as it is a standard feature
and it must be synchronized with the code that is calling the template
(i.e. any variable bindings define a sort of API that the caller uses.
Changing the content also risks breaking that contract, since the caller
could be updated while a customized version of this template will not.).
The only case I could imagine where the reset password email might be
customized would be when there exists no translation for a particular
language and an admin might want to translate it on the fly, however, we
could treat this as a limitation of the current version and improve it in
future versions, just like any other code feature.

Share by email probably also has an email template which should be
"default" as well, since the only customization it is designed to support
is the actual message included by the user and that is already separate
from the template.

Other examples? (Notifications probably has its own mechanism for
displaying certain notifications or what notifications to send by email)

Thanks,
Eduard

On Mon, May 7, 2018 at 7:26 PM, Vincent Massol <[hidden email]> wrote:

>
>
> > On 7 May 2018, at 18:18, Thomas Mortagne <[hidden email]>
> wrote:
> >
> > On Mon, May 7, 2018 at 5:02 PM, Vincent Massol <[hidden email]>
> wrote:
> >>
> >>
> >>> On 7 May 2018, at 16:48, Thomas Mortagne <[hidden email]>
> wrote:
> >>>
> >>> On Mon, May 7, 2018 at 4:33 PM, Vincent Massol <[hidden email]>
> wrote:
> >>>> Hi,
> >>>>
> >>>> It seems we forgot to handle mail template pages. For example
> XWiki.ResetPasswordMailContent
> >>>>
> >>>> We need to decide the type: demo, default, etc.
> >>>>
> >>>> WDYT about demo (i.e. as soon as the user starts modifying it, we
> don’t upgrade it anymore)?
> >>>>
> >>>> Thanks
> >>>> -Vincent
> >>>>
> >>>
> >>> All types with allowed edit prevent upgrade.
> >>
> >> I’m not sure we need more than 1 such type. See other mail thread.
> >>
> >>> I think a more important question is: is it OK to delete it ?
> >>
> >> We could. See below
> >>
> >>>
> >>> Seems to me delete is not OK in this context. Unless it's possible to
> >>> change the mail template used for password reset ?
> >>
> >> Re delete, I think there’s another thread discussing it, no? I don’t
> remember the discussion too well and don’t master all the details but AFAIR
> my preference was to not prevent deletion in general (I’m worried about
> unplanned use cases requiring a delete, like renaming the page to another
> place to save it, and then import some XAR containing the new mail
> template).
> >>
> >> IMO all pages should be deletable without endangering the system. In
> this case we could imagine:
> >> * if the template is missing then the password reset page would mention
> it with the ability to create a default mail template
> >> * and/or report a mail error in the admin UI when sending the email
> (since the template doesn’t exist). This means that the template factory
> for emails should check the existence of the page. This should be handled
> here: https://github.com/xwiki/xwiki-platform/blob/
> 6e281a093d3751666fdcd3fb3a69cb638cca9b59/xwiki-platform-
> core/xwiki-platform-mail/xwiki-platform-mail-send/
> xwiki-platform-mail-send-default/src/main/java/org/
> xwiki/mail/internal/factory/template/DefaultMailTemplateManager.java#L143
> AFAICS it will currently report a NPE….
> >
> > As you said, deleting that page would break reset password feature and
> > since I don't plan to rewrite it right now it means delete should be
> > protected IMO. If someone improve this feature later then the type can
> > be changed to "demo".
> >
> >>
> >> Have we decided what we do about deletes in general?
> >
> > There hasn't been such discussion.
>
> I’m referring to http://markmail.org/message/kjtyzvjp5zzh4gyf (and I’m
> sure I saw another discussion about that but cannot find it ATM).
>
> I still don’t understand why we’re mixing upgradability with deletability
> (and trying to find names that represent both). Aren’t they 2 different
> topics?
>
> Thanks
> -Vincent
>
> > I don't even understand what this
> > mean, it's obvious to me that deleting some pages don't break anything
> > while for others you are going to create a huge mess.
> >
> >>
> >> Thanks
> >> -Vincent
> >>
> >>> --
> >>> Thomas Mortagne
> >>
> >
> >
> >
> > --
> > Thomas Mortagne
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] What XAR file type for Mail Template pages?

vmassol
Administrator
Hi Edy,

> On 8 May 2018, at 12:38, Eduard Moraru <[hidden email]> wrote:
>
> Hi,
>
> IMO, it's just like any other page: it depends on the application it is
> part of and we can't treat them all the same.
>
> XWiki.ResetPasswordMailContent should be "default". I see no particular
> reason to customize the reset password email, as it is a standard feature
> and it must be synchronized with the code that is calling the template
> (i.e. any variable bindings define a sort of API that the caller uses.
> Changing the content also risks breaking that contract, since the caller
> could be updated while a customized version of this template will not.).
> The only case I could imagine where the reset password email might be
> customized would be when there exists no translation for a particular
> language and an admin might want to translate it on the fly, however, we
> could treat this as a limitation of the current version and improve it in
> future versions, just like any other code feature.

For me anything visual (themes, skins and thus email templates) are things that only should be able to be changed but that users will change for sure (and we”ve seen changes for all of them in the history of XWiki).

And yes the bindings we allow using in them are “API contracts” that we shouldn’t break at all. They’re exactly like scripting APIs.

> Share by email probably also has an email template which should be
> "default" as well, since the only customization it is designed to support
> is the actual message included by the user and that is already separate
> from the template.
>
> Other examples? (Notifications probably has its own mechanism for
> displaying certain notifications or what notifications to send by email)

Sure, it’s even documented:
http://extensions.xwiki.org/xwiki/bin/view/Extension/Notifications%20Application/#HCustomizingthenotificationemailtemplate

Same for watchlist:
http://extensions.xwiki.org/xwiki/bin/view/Extension/Watchlist%20Application#HAdministrators:CustomizingtheWatchListemailtemplate

Any template contains UI and as such users will want to change them. That’s 100% guaranteed. I’m not saying that they’ll all change them but some will. They’ll change the wording, they’ll add some custom logo/banner, they’ll make some modifications, adding some links, etc.

Now between “default” and “demo” I don’t know.

There are 2 aspects:
* Aspect 1: If the user makes changes we should keep them for sure. If we also make changes at the same time, I guess the best is to try to merge them, unless we think it’s going to break most of the time but that’s probably not the case.
* Aspect 2: These pages are meant to be modified by the user so when the user edits they shouldn’t get a warning message.

Do we have a type to represent those 2 aspects? ;)

Thanks
-Vincent

>
> Thanks,
> Eduard
>
> On Mon, May 7, 2018 at 7:26 PM, Vincent Massol <[hidden email]> wrote:
>
>>
>>
>>> On 7 May 2018, at 18:18, Thomas Mortagne <[hidden email]>
>> wrote:
>>>
>>> On Mon, May 7, 2018 at 5:02 PM, Vincent Massol <[hidden email]>
>> wrote:
>>>>
>>>>
>>>>> On 7 May 2018, at 16:48, Thomas Mortagne <[hidden email]>
>> wrote:
>>>>>
>>>>> On Mon, May 7, 2018 at 4:33 PM, Vincent Massol <[hidden email]>
>> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> It seems we forgot to handle mail template pages. For example
>> XWiki.ResetPasswordMailContent
>>>>>>
>>>>>> We need to decide the type: demo, default, etc.
>>>>>>
>>>>>> WDYT about demo (i.e. as soon as the user starts modifying it, we
>> don’t upgrade it anymore)?
>>>>>>
>>>>>> Thanks
>>>>>> -Vincent
>>>>>>
>>>>>
>>>>> All types with allowed edit prevent upgrade.
>>>>
>>>> I’m not sure we need more than 1 such type. See other mail thread.
>>>>
>>>>> I think a more important question is: is it OK to delete it ?
>>>>
>>>> We could. See below
>>>>
>>>>>
>>>>> Seems to me delete is not OK in this context. Unless it's possible to
>>>>> change the mail template used for password reset ?
>>>>
>>>> Re delete, I think there’s another thread discussing it, no? I don’t
>> remember the discussion too well and don’t master all the details but AFAIR
>> my preference was to not prevent deletion in general (I’m worried about
>> unplanned use cases requiring a delete, like renaming the page to another
>> place to save it, and then import some XAR containing the new mail
>> template).
>>>>
>>>> IMO all pages should be deletable without endangering the system. In
>> this case we could imagine:
>>>> * if the template is missing then the password reset page would mention
>> it with the ability to create a default mail template
>>>> * and/or report a mail error in the admin UI when sending the email
>> (since the template doesn’t exist). This means that the template factory
>> for emails should check the existence of the page. This should be handled
>> here: https://github.com/xwiki/xwiki-platform/blob/
>> 6e281a093d3751666fdcd3fb3a69cb638cca9b59/xwiki-platform-
>> core/xwiki-platform-mail/xwiki-platform-mail-send/
>> xwiki-platform-mail-send-default/src/main/java/org/
>> xwiki/mail/internal/factory/template/DefaultMailTemplateManager.java#L143
>> AFAICS it will currently report a NPE….
>>>
>>> As you said, deleting that page would break reset password feature and
>>> since I don't plan to rewrite it right now it means delete should be
>>> protected IMO. If someone improve this feature later then the type can
>>> be changed to "demo".
>>>
>>>>
>>>> Have we decided what we do about deletes in general?
>>>
>>> There hasn't been such discussion.
>>
>> I’m referring to http://markmail.org/message/kjtyzvjp5zzh4gyf (and I’m
>> sure I saw another discussion about that but cannot find it ATM).
>>
>> I still don’t understand why we’re mixing upgradability with deletability
>> (and trying to find names that represent both). Aren’t they 2 different
>> topics?
>>
>> Thanks
>> -Vincent
>>
>>> I don't even understand what this
>>> mean, it's obvious to me that deleting some pages don't break anything
>>> while for others you are going to create a huge mess.
>>>
>>>>
>>>> Thanks
>>>> -Vincent
>>>>
>>>>> --
>>>>> Thomas Mortagne
>>>>
>>>
>>>
>>>
>>> --
>>> Thomas Mortagne
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] What XAR file type for Mail Template pages?

Thomas Mortagne
Administrator
On Fri, May 11, 2018 at 11:00 AM, Vincent Massol <[hidden email]> wrote:

> Hi Edy,
>
>> On 8 May 2018, at 12:38, Eduard Moraru <[hidden email]> wrote:
>>
>> Hi,
>>
>> IMO, it's just like any other page: it depends on the application it is
>> part of and we can't treat them all the same.
>>
>> XWiki.ResetPasswordMailContent should be "default". I see no particular
>> reason to customize the reset password email, as it is a standard feature
>> and it must be synchronized with the code that is calling the template
>> (i.e. any variable bindings define a sort of API that the caller uses.
>> Changing the content also risks breaking that contract, since the caller
>> could be updated while a customized version of this template will not.).
>> The only case I could imagine where the reset password email might be
>> customized would be when there exists no translation for a particular
>> language and an admin might want to translate it on the fly, however, we
>> could treat this as a limitation of the current version and improve it in
>> future versions, just like any other code feature.
>
> For me anything visual (themes, skins and thus email templates) are things that only should be able to be changed but that users will change for sure (and we”ve seen changes for all of them in the history of XWiki).

"anything visual" contains much more than themes, skins and email templates.

>
> And yes the bindings we allow using in them are “API contracts” that we shouldn’t break at all. They’re exactly like scripting APIs.
>
>> Share by email probably also has an email template which should be
>> "default" as well, since the only customization it is designed to support
>> is the actual message included by the user and that is already separate
>> from the template.
>>
>> Other examples? (Notifications probably has its own mechanism for
>> displaying certain notifications or what notifications to send by email)
>
> Sure, it’s even documented:
> http://extensions.xwiki.org/xwiki/bin/view/Extension/Notifications%20Application/#HCustomizingthenotificationemailtemplate
>
> Same for watchlist:
> http://extensions.xwiki.org/xwiki/bin/view/Extension/Watchlist%20Application#HAdministrators:CustomizingtheWatchListemailtemplate

IMO those documentations don't really make those templates
specifically designed to be customized, it's just "if you really need
to customize them that's the only way".

>
> Any template contains UI and as such users will want to change them. That’s 100% guaranteed. I’m not saying that they’ll all change them but some will. They’ll change the wording, they’ll add some custom logo/banner, they’ll make some modifications, adding some links, etc.
>
> Now between “default” and “demo” I don’t know.
>
> There are 2 aspects:
> * Aspect 1: If the user makes changes we should keep them for sure. If we also make changes at the same time, I guess the best is to try to merge them, unless we think it’s going to break most of the time but that’s probably not the case.
> * Aspect 2: These pages are meant to be modified by the user so when the user edits they shouldn’t get a warning message.

I don't think "maybe the user will need to modify this document for
some reason" is the right criteria, IMO it should be
dangerous=warning. For me there is a big difference between "this page
is meant to be modified" and "this page is the only way to modify how
the email looks".

>
> Do we have a type to represent those 2 aspects? ;)

We can have any type we want, just need a name. You seems to be asking
for a type with allowed edit and the rest being the default behavior.

>
> Thanks
> -Vincent
>
>>
>> Thanks,
>> Eduard
>>
>> On Mon, May 7, 2018 at 7:26 PM, Vincent Massol <[hidden email]> wrote:
>>
>>>
>>>
>>>> On 7 May 2018, at 18:18, Thomas Mortagne <[hidden email]>
>>> wrote:
>>>>
>>>> On Mon, May 7, 2018 at 5:02 PM, Vincent Massol <[hidden email]>
>>> wrote:
>>>>>
>>>>>
>>>>>> On 7 May 2018, at 16:48, Thomas Mortagne <[hidden email]>
>>> wrote:
>>>>>>
>>>>>> On Mon, May 7, 2018 at 4:33 PM, Vincent Massol <[hidden email]>
>>> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> It seems we forgot to handle mail template pages. For example
>>> XWiki.ResetPasswordMailContent
>>>>>>>
>>>>>>> We need to decide the type: demo, default, etc.
>>>>>>>
>>>>>>> WDYT about demo (i.e. as soon as the user starts modifying it, we
>>> don’t upgrade it anymore)?
>>>>>>>
>>>>>>> Thanks
>>>>>>> -Vincent
>>>>>>>
>>>>>>
>>>>>> All types with allowed edit prevent upgrade.
>>>>>
>>>>> I’m not sure we need more than 1 such type. See other mail thread.
>>>>>
>>>>>> I think a more important question is: is it OK to delete it ?
>>>>>
>>>>> We could. See below
>>>>>
>>>>>>
>>>>>> Seems to me delete is not OK in this context. Unless it's possible to
>>>>>> change the mail template used for password reset ?
>>>>>
>>>>> Re delete, I think there’s another thread discussing it, no? I don’t
>>> remember the discussion too well and don’t master all the details but AFAIR
>>> my preference was to not prevent deletion in general (I’m worried about
>>> unplanned use cases requiring a delete, like renaming the page to another
>>> place to save it, and then import some XAR containing the new mail
>>> template).
>>>>>
>>>>> IMO all pages should be deletable without endangering the system. In
>>> this case we could imagine:
>>>>> * if the template is missing then the password reset page would mention
>>> it with the ability to create a default mail template
>>>>> * and/or report a mail error in the admin UI when sending the email
>>> (since the template doesn’t exist). This means that the template factory
>>> for emails should check the existence of the page. This should be handled
>>> here: https://github.com/xwiki/xwiki-platform/blob/
>>> 6e281a093d3751666fdcd3fb3a69cb638cca9b59/xwiki-platform-
>>> core/xwiki-platform-mail/xwiki-platform-mail-send/
>>> xwiki-platform-mail-send-default/src/main/java/org/
>>> xwiki/mail/internal/factory/template/DefaultMailTemplateManager.java#L143
>>> AFAICS it will currently report a NPE….
>>>>
>>>> As you said, deleting that page would break reset password feature and
>>>> since I don't plan to rewrite it right now it means delete should be
>>>> protected IMO. If someone improve this feature later then the type can
>>>> be changed to "demo".
>>>>
>>>>>
>>>>> Have we decided what we do about deletes in general?
>>>>
>>>> There hasn't been such discussion.
>>>
>>> I’m referring to http://markmail.org/message/kjtyzvjp5zzh4gyf (and I’m
>>> sure I saw another discussion about that but cannot find it ATM).
>>>
>>> I still don’t understand why we’re mixing upgradability with deletability
>>> (and trying to find names that represent both). Aren’t they 2 different
>>> topics?
>>>
>>> Thanks
>>> -Vincent
>>>
>>>> I don't even understand what this
>>>> mean, it's obvious to me that deleting some pages don't break anything
>>>> while for others you are going to create a huge mess.
>>>>
>>>>>
>>>>> Thanks
>>>>> -Vincent
>>>>>
>>>>>> --
>>>>>> Thomas Mortagne
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Thomas Mortagne
>>>
>>>
>



--
Thomas Mortagne
Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] What XAR file type for Mail Template pages?

vmassol
Administrator


> On 11 May 2018, at 14:22, Thomas Mortagne <[hidden email]> wrote:
>
> On Fri, May 11, 2018 at 11:00 AM, Vincent Massol <[hidden email]> wrote:
>> Hi Edy,
>>
>>> On 8 May 2018, at 12:38, Eduard Moraru <[hidden email]> wrote:
>>>
>>> Hi,
>>>
>>> IMO, it's just like any other page: it depends on the application it is
>>> part of and we can't treat them all the same.
>>>
>>> XWiki.ResetPasswordMailContent should be "default". I see no particular
>>> reason to customize the reset password email, as it is a standard feature
>>> and it must be synchronized with the code that is calling the template
>>> (i.e. any variable bindings define a sort of API that the caller uses.
>>> Changing the content also risks breaking that contract, since the caller
>>> could be updated while a customized version of this template will not.).
>>> The only case I could imagine where the reset password email might be
>>> customized would be when there exists no translation for a particular
>>> language and an admin might want to translate it on the fly, however, we
>>> could treat this as a limitation of the current version and improve it in
>>> future versions, just like any other code feature.
>>
>> For me anything visual (themes, skins and thus email templates) are things that only should be able to be changed but that users will change for sure (and we”ve seen changes for all of them in the history of XWiki).
>
> "anything visual" contains much more than themes, skins and email templates.
>
>>
>> And yes the bindings we allow using in them are “API contracts” that we shouldn’t break at all. They’re exactly like scripting APIs.
>>
>>> Share by email probably also has an email template which should be
>>> "default" as well, since the only customization it is designed to support
>>> is the actual message included by the user and that is already separate
>>> from the template.
>>>
>>> Other examples? (Notifications probably has its own mechanism for
>>> displaying certain notifications or what notifications to send by email)
>>
>> Sure, it’s even documented:
>> http://extensions.xwiki.org/xwiki/bin/view/Extension/Notifications%20Application/#HCustomizingthenotificationemailtemplate
>>
>> Same for watchlist:
>> http://extensions.xwiki.org/xwiki/bin/view/Extension/Watchlist%20Application#HAdministrators:CustomizingtheWatchListemailtemplate
>
> IMO those documentations don't really make those templates
> specifically designed to be customized, it's just "if you really need
> to customize them that's the only way”.

No, they’re meant to be customized and modified.

>
>>
>> Any template contains UI and as such users will want to change them. That’s 100% guaranteed. I’m not saying that they’ll all change them but some will. They’ll change the wording, they’ll add some custom logo/banner, they’ll make some modifications, adding some links, etc.
>>
>> Now between “default” and “demo” I don’t know.
>>
>> There are 2 aspects:
>> * Aspect 1: If the user makes changes we should keep them for sure. If we also make changes at the same time, I guess the best is to try to merge them, unless we think it’s going to break most of the time but that’s probably not the case.
>> * Aspect 2: These pages are meant to be modified by the user so when the user edits they shouldn’t get a warning message.
>
> I don't think "maybe the user will need to modify this document for
> some reason" is the right criteria, IMO it should be
> dangerous=warning. For me there is a big difference between "this page
> is meant to be modified" and "this page is the only way to modify how
> the email looks”.

Users should not get a warning for them since they’re supposed to be edited to customize the email templates.

I disagree with dangerous = warning because if you read the warning message, it says that the page shouldn’t be edited because it belongs to an extension. Oviousldy this message doesn’t apply here. Unless you’re talking about another warning message specific to this type?

>
>> Do we have a type to represent those 2 aspects? ;)
>
> We can have any type we want, just need a name. You seems to be asking
> for a type with allowed edit and the rest being the default behavior.

“user”? (as in “it’s a user page”)
“editable”? (as in “it’s an extension page supposed to be editable by users)

But first let’s agree about the type/need.

Thanks
-Vincent

>
>>
>> Thanks
>> -Vincent
>>
>>>
>>> Thanks,
>>> Eduard
>>>
>>> On Mon, May 7, 2018 at 7:26 PM, Vincent Massol <[hidden email]> wrote:
>>>
>>>>
>>>>
>>>>> On 7 May 2018, at 18:18, Thomas Mortagne <[hidden email]>
>>>> wrote:
>>>>>
>>>>> On Mon, May 7, 2018 at 5:02 PM, Vincent Massol <[hidden email]>
>>>> wrote:
>>>>>>
>>>>>>
>>>>>>> On 7 May 2018, at 16:48, Thomas Mortagne <[hidden email]>
>>>> wrote:
>>>>>>>
>>>>>>> On Mon, May 7, 2018 at 4:33 PM, Vincent Massol <[hidden email]>
>>>> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> It seems we forgot to handle mail template pages. For example
>>>> XWiki.ResetPasswordMailContent
>>>>>>>>
>>>>>>>> We need to decide the type: demo, default, etc.
>>>>>>>>
>>>>>>>> WDYT about demo (i.e. as soon as the user starts modifying it, we
>>>> don’t upgrade it anymore)?
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> -Vincent
>>>>>>>>
>>>>>>>
>>>>>>> All types with allowed edit prevent upgrade.
>>>>>>
>>>>>> I’m not sure we need more than 1 such type. See other mail thread.
>>>>>>
>>>>>>> I think a more important question is: is it OK to delete it ?
>>>>>>
>>>>>> We could. See below
>>>>>>
>>>>>>>
>>>>>>> Seems to me delete is not OK in this context. Unless it's possible to
>>>>>>> change the mail template used for password reset ?
>>>>>>
>>>>>> Re delete, I think there’s another thread discussing it, no? I don’t
>>>> remember the discussion too well and don’t master all the details but AFAIR
>>>> my preference was to not prevent deletion in general (I’m worried about
>>>> unplanned use cases requiring a delete, like renaming the page to another
>>>> place to save it, and then import some XAR containing the new mail
>>>> template).
>>>>>>
>>>>>> IMO all pages should be deletable without endangering the system. In
>>>> this case we could imagine:
>>>>>> * if the template is missing then the password reset page would mention
>>>> it with the ability to create a default mail template
>>>>>> * and/or report a mail error in the admin UI when sending the email
>>>> (since the template doesn’t exist). This means that the template factory
>>>> for emails should check the existence of the page. This should be handled
>>>> here: https://github.com/xwiki/xwiki-platform/blob/
>>>> 6e281a093d3751666fdcd3fb3a69cb638cca9b59/xwiki-platform-
>>>> core/xwiki-platform-mail/xwiki-platform-mail-send/
>>>> xwiki-platform-mail-send-default/src/main/java/org/
>>>> xwiki/mail/internal/factory/template/DefaultMailTemplateManager.java#L143
>>>> AFAICS it will currently report a NPE….
>>>>>
>>>>> As you said, deleting that page would break reset password feature and
>>>>> since I don't plan to rewrite it right now it means delete should be
>>>>> protected IMO. If someone improve this feature later then the type can
>>>>> be changed to "demo".
>>>>>
>>>>>>
>>>>>> Have we decided what we do about deletes in general?
>>>>>
>>>>> There hasn't been such discussion.
>>>>
>>>> I’m referring to http://markmail.org/message/kjtyzvjp5zzh4gyf (and I’m
>>>> sure I saw another discussion about that but cannot find it ATM).
>>>>
>>>> I still don’t understand why we’re mixing upgradability with deletability
>>>> (and trying to find names that represent both). Aren’t they 2 different
>>>> topics?
>>>>
>>>> Thanks
>>>> -Vincent
>>>>
>>>>> I don't even understand what this
>>>>> mean, it's obvious to me that deleting some pages don't break anything
>>>>> while for others you are going to create a huge mess.
>>>>>
>>>>>>
>>>>>> Thanks
>>>>>> -Vincent
>>>>>>
>>>>>>> --
>>>>>>> Thomas Mortagne
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Thomas Mortagne
>>>>
>>>>
>>
>
>
>
> --
> Thomas Mortagne

Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] What XAR file type for Mail Template pages?

Eduard Moraru
Hi,

On Fri, May 11, 2018 at 7:21 PM, Vincent Massol <[hidden email]> wrote:

>
>
> > On 11 May 2018, at 14:22, Thomas Mortagne <[hidden email]>
> wrote:
> >
> > On Fri, May 11, 2018 at 11:00 AM, Vincent Massol <[hidden email]>
> wrote:
> >> Hi Edy,
> >>
> >>> On 8 May 2018, at 12:38, Eduard Moraru <[hidden email]> wrote:
> >>>
> >>> Hi,
> >>>
> >>> IMO, it's just like any other page: it depends on the application it is
> >>> part of and we can't treat them all the same.
> >>>
> >>> XWiki.ResetPasswordMailContent should be "default". I see no particular
> >>> reason to customize the reset password email, as it is a standard
> feature
> >>> and it must be synchronized with the code that is calling the template
> >>> (i.e. any variable bindings define a sort of API that the caller uses.
> >>> Changing the content also risks breaking that contract, since the
> caller
> >>> could be updated while a customized version of this template will
> not.).
> >>> The only case I could imagine where the reset password email might be
> >>> customized would be when there exists no translation for a particular
> >>> language and an admin might want to translate it on the fly, however,
> we
> >>> could treat this as a limitation of the current version and improve it
> in
> >>> future versions, just like any other code feature.
> >>
> >> For me anything visual (themes, skins and thus email templates) are
> things that only should be able to be changed but that users will change
> for sure (and we”ve seen changes for all of them in the history of XWiki).
> >
> > "anything visual" contains much more than themes, skins and email
> templates.
> >
> >>
> >> And yes the bindings we allow using in them are “API contracts” that we
> shouldn’t break at all. They’re exactly like scripting APIs.
> >>
> >>> Share by email probably also has an email template which should be
> >>> "default" as well, since the only customization it is designed to
> support
> >>> is the actual message included by the user and that is already separate
> >>> from the template.
> >>>
> >>> Other examples? (Notifications probably has its own mechanism for
> >>> displaying certain notifications or what notifications to send by
> email)
> >>
> >> Sure, it’s even documented:
> >> http://extensions.xwiki.org/xwiki/bin/view/Extension/
> Notifications%20Application/#HCustomizingthenotificationemailtemplate
> >>
> >> Same for watchlist:
> >> http://extensions.xwiki.org/xwiki/bin/view/Extension/
> Watchlist%20Application#HAdministrators:CustomizingtheWatchListemailte
> mplate
> >
> > IMO those documentations don't really make those templates
> > specifically designed to be customized, it's just "if you really need
> > to customize them that's the only way”.
>
> No, they’re meant to be customized and modified.
>

I'm sorry, but I personally don not agree with this point of view at all.

This may be the way the Mail API works, with a template page that is meant
to be customized to be able to make up the content of the email. However,
when an application chooses to use the mail API, it is forced to use the
same mechanism, but that does not also come with the obligation for the
resulting email to be customized by the user, just because it's relatively
easy for the user to do so, through a relatively well known mechanism. IMO,
our target with the "disallow editing" effort is to identify pages that
contain code (including code and content) that are part of an
application/extension and make sure users don't start simply editing them,
for whatever customization reason they or we may come up with. These
particular 2 mail templates do contain a lot of code and are subject to
improvement. The fact that both are document is probably a consequence of
Notifications trying to offer the same level of features/doc as the
previous watchlist feature.

I am still advocating for the copy and customize approach, as we need to
clearly identify what is standard and what is the user's customization and
we need to limit as much as possible the number of automatic merges
(possible point of failure) that are to be done during an upgrade. If we
want to make the Notifications email customizable, we should add a
configuration option of the Notifications feature that allows an admin to
provide a custom email template. IMO, that is the clear and clean solution
that we should strive for.

As an exercise, Try to visualize the likelihood of a customized
Notifications (i.e. still under development) email template surviving for
3-5 years later on, specially if we go it the previously suggested approach
of not overriding or merging at all, because the user customized it.

Thanks,
Eduard


>
> >
> >>
> >> Any template contains UI and as such users will want to change them.
> That’s 100% guaranteed. I’m not saying that they’ll all change them but
> some will. They’ll change the wording, they’ll add some custom logo/banner,
> they’ll make some modifications, adding some links, etc.
> >>
> >> Now between “default” and “demo” I don’t know.
> >>
> >> There are 2 aspects:
> >> * Aspect 1: If the user makes changes we should keep them for sure. If
> we also make changes at the same time, I guess the best is to try to merge
> them, unless we think it’s going to break most of the time but that’s
> probably not the case.
> >> * Aspect 2: These pages are meant to be modified by the user so when
> the user edits they shouldn’t get a warning message.
> >
> > I don't think "maybe the user will need to modify this document for
> > some reason" is the right criteria, IMO it should be
> > dangerous=warning. For me there is a big difference between "this page
> > is meant to be modified" and "this page is the only way to modify how
> > the email looks”.
>
> Users should not get a warning for them since they’re supposed to be
> edited to customize the email templates.
>
> I disagree with dangerous = warning because if you read the warning
> message, it says that the page shouldn’t be edited because it belongs to an
> extension. Oviousldy this message doesn’t apply here. Unless you’re talking
> about another warning message specific to this type?
>
> >
> >> Do we have a type to represent those 2 aspects? ;)
> >
> > We can have any type we want, just need a name. You seems to be asking
> > for a type with allowed edit and the rest being the default behavior.
>
> “user”? (as in “it’s a user page”)
> “editable”? (as in “it’s an extension page supposed to be editable by
> users)
>
> But first let’s agree about the type/need.
>
> Thanks
> -Vincent
>
> >
> >>
> >> Thanks
> >> -Vincent
> >>
> >>>
> >>> Thanks,
> >>> Eduard
> >>>
> >>> On Mon, May 7, 2018 at 7:26 PM, Vincent Massol <[hidden email]>
> wrote:
> >>>
> >>>>
> >>>>
> >>>>> On 7 May 2018, at 18:18, Thomas Mortagne <[hidden email]>
> >>>> wrote:
> >>>>>
> >>>>> On Mon, May 7, 2018 at 5:02 PM, Vincent Massol <[hidden email]>
> >>>> wrote:
> >>>>>>
> >>>>>>
> >>>>>>> On 7 May 2018, at 16:48, Thomas Mortagne <
> [hidden email]>
> >>>> wrote:
> >>>>>>>
> >>>>>>> On Mon, May 7, 2018 at 4:33 PM, Vincent Massol <[hidden email]
> >
> >>>> wrote:
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> It seems we forgot to handle mail template pages. For example
> >>>> XWiki.ResetPasswordMailContent
> >>>>>>>>
> >>>>>>>> We need to decide the type: demo, default, etc.
> >>>>>>>>
> >>>>>>>> WDYT about demo (i.e. as soon as the user starts modifying it, we
> >>>> don’t upgrade it anymore)?
> >>>>>>>>
> >>>>>>>> Thanks
> >>>>>>>> -Vincent
> >>>>>>>>
> >>>>>>>
> >>>>>>> All types with allowed edit prevent upgrade.
> >>>>>>
> >>>>>> I’m not sure we need more than 1 such type. See other mail thread.
> >>>>>>
> >>>>>>> I think a more important question is: is it OK to delete it ?
> >>>>>>
> >>>>>> We could. See below
> >>>>>>
> >>>>>>>
> >>>>>>> Seems to me delete is not OK in this context. Unless it's possible
> to
> >>>>>>> change the mail template used for password reset ?
> >>>>>>
> >>>>>> Re delete, I think there’s another thread discussing it, no? I don’t
> >>>> remember the discussion too well and don’t master all the details but
> AFAIR
> >>>> my preference was to not prevent deletion in general (I’m worried
> about
> >>>> unplanned use cases requiring a delete, like renaming the page to
> another
> >>>> place to save it, and then import some XAR containing the new mail
> >>>> template).
> >>>>>>
> >>>>>> IMO all pages should be deletable without endangering the system. In
> >>>> this case we could imagine:
> >>>>>> * if the template is missing then the password reset page would
> mention
> >>>> it with the ability to create a default mail template
> >>>>>> * and/or report a mail error in the admin UI when sending the email
> >>>> (since the template doesn’t exist). This means that the template
> factory
> >>>> for emails should check the existence of the page. This should be
> handled
> >>>> here: https://github.com/xwiki/xwiki-platform/blob/
> >>>> 6e281a093d3751666fdcd3fb3a69cb638cca9b59/xwiki-platform-
> >>>> core/xwiki-platform-mail/xwiki-platform-mail-send/
> >>>> xwiki-platform-mail-send-default/src/main/java/org/
> >>>> xwiki/mail/internal/factory/template/DefaultMailTemplateManager.
> java#L143
> >>>> AFAICS it will currently report a NPE….
> >>>>>
> >>>>> As you said, deleting that page would break reset password feature
> and
> >>>>> since I don't plan to rewrite it right now it means delete should be
> >>>>> protected IMO. If someone improve this feature later then the type
> can
> >>>>> be changed to "demo".
> >>>>>
> >>>>>>
> >>>>>> Have we decided what we do about deletes in general?
> >>>>>
> >>>>> There hasn't been such discussion.
> >>>>
> >>>> I’m referring to http://markmail.org/message/kjtyzvjp5zzh4gyf (and
> I’m
> >>>> sure I saw another discussion about that but cannot find it ATM).
> >>>>
> >>>> I still don’t understand why we’re mixing upgradability with
> deletability
> >>>> (and trying to find names that represent both). Aren’t they 2
> different
> >>>> topics?
> >>>>
> >>>> Thanks
> >>>> -Vincent
> >>>>
> >>>>> I don't even understand what this
> >>>>> mean, it's obvious to me that deleting some pages don't break
> anything
> >>>>> while for others you are going to create a huge mess.
> >>>>>
> >>>>>>
> >>>>>> Thanks
> >>>>>> -Vincent
> >>>>>>
> >>>>>>> --
> >>>>>>> Thomas Mortagne
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> Thomas Mortagne
> >>>>
> >>>>
> >>
> >
> >
> >
> > --
> > Thomas Mortagne
>
>