On Fri, May 11, 2018 at 11:00 AM, Vincent Massol <[email protected]> wrote: > Hi Edy, > >> On 8 May 2018, at 12:38, Eduard Moraru <[email protected]> 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 <[email protected]> wrote: >> >>> >>> >>>> On 7 May 2018, at 18:18, Thomas Mortagne <[email protected]> >>> wrote: >>>> >>>> On Mon, May 7, 2018 at 5:02 PM, Vincent Massol <[email protected]> >>> wrote: >>>>> >>>>> >>>>>> On 7 May 2018, at 16:48, Thomas Mortagne <[email protected]> >>> wrote: >>>>>> >>>>>> On Mon, May 7, 2018 at 4:33 PM, Vincent Massol <[email protected]> >>> 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
