[jira] Created: (OFBIZ-1118) Pagination enhancement (paginate-target issue)

[Nicolas Malin (Jira)]

Pagination enhancement (paginate-target issue)
----------------------------------------------

                 Key: OFBIZ-1118
                 URL: https://issues.apache.org/jira/browse/OFBIZ-1118
             Project: OFBiz
          Issue Type: Improvement
    Affects Versions: SVN trunk
            Reporter: Wickersheimer Jeremy
            Priority: Minor
             Fix For: SVN trunk


Currently Form pagination is using either the "target" attribute or the "paginate-target" attribute from the Form definition.

After having hacked a few Forms and since i implemented another feature using this value  ( column sorting ) i found out that
- "target" value is seldom correct for pagination, it is very often used for update / delete links from the list
- "paginate-target" is not really useful value because:
--- if the form is used on multiple screen (therefore multiple URLs) it cannot be used and pagination should be disabled.
--- if the form is used on one page only we have to repeat the target URL in the form which is taken from the Screen -> controller. Basically we are backtracking from the Form to the controller and coupling them.

I don't have any example in mind were paginate-target would be different from the request URL and i often fixed missing paginate-target by copy pasting the part from the URL.

So rather than running after the legion of forms that has a target but no paginate-target, those that have no paginate-target at all and those where pagination would be buggy because more than one form is on the same screen we can simply change the logic a bit.
- we use "paginate-target" if explicitly set in the form
- else we do not fall back to "target" bu to the request URL (from context parameters targetRequestUri)

This is very simply done by patching the getPaginateTarget for the ModelForm. I won't attach a patch for now, just the code (simple anyway):

public String getPaginateTarget(Map context) {
        String targ = this.paginateTarget.expandString(context);
        if (UtilValidate.isEmpty(targ)) {
            Map parameters = (Map) context.get("parameters");
            if(parameters != null && parameters.containsKey("targetRequestUri")) {
                targ = (String) parameters.get("targetRequestUri");
            }
        }
        return targ;
    }


Is there anyone that thinks it is a bad idea .. or the reason why it wasn't done like this before ?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-1118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12527707 ]

Jacopo Cappellato commented on OFBIZ-1118:
------------------------------------------

Still not tested, but I think it is a good idea; are there any objections around this?

Jacopo

> Pagination enhancement (paginate-target issue)
> ----------------------------------------------
>
>                 Key: OFBIZ-1118
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-1118
>             Project: OFBiz
>          Issue Type: Improvement
>    Affects Versions: SVN trunk
>            Reporter: Wickersheimer Jeremy
>            Priority: Minor
>             Fix For: SVN trunk
>
>
> Currently Form pagination is using either the "target" attribute or the "paginate-target" attribute from the Form definition.
> After having hacked a few Forms and since i implemented another feature using this value  ( column sorting ) i found out that
> - "target" value is seldom correct for pagination, it is very often used for update / delete links from the list
> - "paginate-target" is not really useful value because:
> --- if the form is used on multiple screen (therefore multiple URLs) it cannot be used and pagination should be disabled.
> --- if the form is used on one page only we have to repeat the target URL in the form which is taken from the Screen -> controller. Basically we are backtracking from the Form to the controller and coupling them.
> I don't have any example in mind were paginate-target would be different from the request URL and i often fixed missing paginate-target by copy pasting the part from the URL.
> So rather than running after the legion of forms that has a target but no paginate-target, those that have no paginate-target at all and those where pagination would be buggy because more than one form is on the same screen we can simply change the logic a bit.
> - we use "paginate-target" if explicitly set in the form
> - else we do not fall back to "target" bu to the request URL (from context parameters targetRequestUri)
> This is very simply done by patching the getPaginateTarget for the ModelForm. I won't attach a patch for now, just the code (simple anyway):
> public String getPaginateTarget(Map context) {
>         String targ = this.paginateTarget.expandString(context);
>         if (UtilValidate.isEmpty(targ)) {
>             Map parameters = (Map) context.get("parameters");
>             if(parameters != null && parameters.containsKey("targetRequestUri")) {
>                 targ = (String) parameters.get("targetRequestUri");
>             }
>         }
>         return targ;
>     }
> Is there anyone that thinks it is a bad idea .. or the reason why it wasn't done like this before ?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-1118?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacopo Cappellato reassigned OFBIZ-1118:
----------------------------------------

    Assignee: Jacopo Cappellato

> Pagination enhancement (paginate-target issue)
> ----------------------------------------------
>
>                 Key: OFBIZ-1118
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-1118
>             Project: OFBiz
>          Issue Type: Improvement
>    Affects Versions: SVN trunk
>            Reporter: Wickersheimer Jeremy
>            Assignee: Jacopo Cappellato
>            Priority: Minor
>             Fix For: SVN trunk
>
>
> Currently Form pagination is using either the "target" attribute or the "paginate-target" attribute from the Form definition.
> After having hacked a few Forms and since i implemented another feature using this value  ( column sorting ) i found out that
> - "target" value is seldom correct for pagination, it is very often used for update / delete links from the list
> - "paginate-target" is not really useful value because:
> --- if the form is used on multiple screen (therefore multiple URLs) it cannot be used and pagination should be disabled.
> --- if the form is used on one page only we have to repeat the target URL in the form which is taken from the Screen -> controller. Basically we are backtracking from the Form to the controller and coupling them.
> I don't have any example in mind were paginate-target would be different from the request URL and i often fixed missing paginate-target by copy pasting the part from the URL.
> So rather than running after the legion of forms that has a target but no paginate-target, those that have no paginate-target at all and those where pagination would be buggy because more than one form is on the same screen we can simply change the logic a bit.
> - we use "paginate-target" if explicitly set in the form
> - else we do not fall back to "target" bu to the request URL (from context parameters targetRequestUri)
> This is very simply done by patching the getPaginateTarget for the ModelForm. I won't attach a patch for now, just the code (simple anyway):
> public String getPaginateTarget(Map context) {
>         String targ = this.paginateTarget.expandString(context);
>         if (UtilValidate.isEmpty(targ)) {
>             Map parameters = (Map) context.get("parameters");
>             if(parameters != null && parameters.containsKey("targetRequestUri")) {
>                 targ = (String) parameters.get("targetRequestUri");
>             }
>         }
>         return targ;
>     }
> Is there anyone that thinks it is a bad idea .. or the reason why it wasn't done like this before ?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-1118?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacopo Cappellato closed OFBIZ-1118.
------------------------------------

    Resolution: Fixed

Thanks Jeremy,

this sounds like a great improvement. Rev. 586449

> Pagination enhancement (paginate-target issue)
> ----------------------------------------------
>
>                 Key: OFBIZ-1118
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-1118
>             Project: OFBiz
>          Issue Type: Improvement
>    Affects Versions: SVN trunk
>            Reporter: Wickersheimer Jeremy
>            Assignee: Jacopo Cappellato
>            Priority: Minor
>             Fix For: SVN trunk
>
>
> Currently Form pagination is using either the "target" attribute or the "paginate-target" attribute from the Form definition.
> After having hacked a few Forms and since i implemented another feature using this value  ( column sorting ) i found out that
> - "target" value is seldom correct for pagination, it is very often used for update / delete links from the list
> - "paginate-target" is not really useful value because:
> --- if the form is used on multiple screen (therefore multiple URLs) it cannot be used and pagination should be disabled.
> --- if the form is used on one page only we have to repeat the target URL in the form which is taken from the Screen -> controller. Basically we are backtracking from the Form to the controller and coupling them.
> I don't have any example in mind were paginate-target would be different from the request URL and i often fixed missing paginate-target by copy pasting the part from the URL.
> So rather than running after the legion of forms that has a target but no paginate-target, those that have no paginate-target at all and those where pagination would be buggy because more than one form is on the same screen we can simply change the logic a bit.
> - we use "paginate-target" if explicitly set in the form
> - else we do not fall back to "target" bu to the request URL (from context parameters targetRequestUri)
> This is very simply done by patching the getPaginateTarget for the ModelForm. I won't attach a patch for now, just the code (simple anyway):
> public String getPaginateTarget(Map context) {
>         String targ = this.paginateTarget.expandString(context);
>         if (UtilValidate.isEmpty(targ)) {
>             Map parameters = (Map) context.get("parameters");
>             if(parameters != null && parameters.containsKey("targetRequestUri")) {
>                 targ = (String) parameters.get("targetRequestUri");
>             }
>         }
>         return targ;
>     }
> Is there anyone that thinks it is a bad idea .. or the reason why it wasn't done like this before ?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.