Menu ▾ ▴

#2149 Tax Rate Parent window not working

Beta
closed-works-for-me
Kai Schaeffer
Taxes (17)
9
2009-12-24
2009-09-30
Carlos Ruiz
No

Test case:

Open Tax Rate Parent window:

WRONG -> The "GST/PST" parent tax is shown WITHOUT DETAILS

Search all taxes:

RIGHT -> The "GST/PST" is shown - and the two details are shown

Search taxes on the detail tab:

WRONG -> no taxes are found.

POSSIBLE CAUSE:

This window is special in that master and detail tabs are based on the same C_Tax table.

The tax children tab on this window has this WhereClause condition:

C_Tax.IsSummary='N' AND C_Tax.Parent_Tax_ID=@C_Tax_ID@

@C_Tax_ID@ is intended to take the value of the parent tab, although as the children tab has the same column then the variable is filled unproperly.

Regards,

Carlos Ruiz

Discussion

  • Kai Schaeffer

    Kai Schaeffer - 2009-10-29
    • assigned_to: nobody --> kai7
    • status: open --> open-accepted
     

  • Kai Schaeffer

    Kai Schaeffer - 2009-10-29

    ANALYSIS:

    The fields of the where clause are replaced by values in the context. The context keys have the form: <WindowNo>|<ColumnName>

    So there is no separation of same column names in different tabs in one window. So the context value of C_Tax_ID is overwritten by the main tab or the detail tab depending on who came last. One easy way to see it, is to click on the refresh button of the main toolbar and then of the detail toolbar and on the main and on the detail... The details appear and disappear and appear. and disappear....

    SOLUTION:

    There is a simple solution (see patch for GridField.java class). I just set an additional context value which also includes the TabNo. The key has the format <WindowNo>|<TabNo>|<ColumnName>. Since I do it additional to the old value there is no breaking of the old functionality. Now you could also specify the TabNo from where you want to reference the column name in the where clause.

    Because of the format of the context key there is no need to change the Env.parseContext function. Just enter for the WhereClause:

    C_Tax.IsSummary='N' AND C_Tax.Parent_Tax_ID=@0|C_Tax_ID@

    and everything works as it should.

    -Kai

     

  • Kai Schaeffer

    Kai Schaeffer - 2009-10-29
     
    GridField.patch

  • Kai Schaeffer

    Kai Schaeffer - 2009-10-29
    • status: open-accepted --> open-fixed
     

  • Kai Schaeffer

    Kai Schaeffer - 2009-10-29

    Oh I forgot:

    DISADVANTAGE:
    Every column is now twice in the context. So the context growth noteworthy in general to cover this single case. An idea would be to have it as an option for the window in the AD.

     

  • Carlos Ruiz

    Carlos Ruiz - 2009-10-29

    Kai, we commonly use "Remind" resolution for things with proposed solutions pending to review.

    About your patch, you're right, the context is duplicated (window context + tab context now).

    At least for this patch this could be solved simply adding the primary key to the context. But I think this is useful also for "Processed" and "IsActive" fields. Many times I've seen customized windows (and not customized also) working unproperly because they take the "Processed" value from the master tab instead of the detail tab. Your proposed solution can solve also this problem.

    So, being conservative I think we could add to Tab Context the fields KeyColumn (key column of the table on the tab), Processed and IsActive.

    WDYT?

    Regards,

    Carlos Ruiz

     

  • Carlos Ruiz

    Carlos Ruiz - 2009-10-29
    • status: open-fixed --> open-remind
     

  • Kai Schaeffer

    Kai Schaeffer - 2009-10-30

    Carlos,
    Sorry for using the wrong resolution. I will keep it in mind for the next bug day. :-)

    Yes, limiting it to some specific columns could solve the growth problem. This seems to be a good and easy compromise. The question is, if there are some cases where it would be helpful to have it for other fields. I guess than it had to be configurable (on the field level in the AD?).

    However, we maybe could start with your suggested fields and see if it comes up for other.

    Regards,
    Kai

     

  • Victor Perez Juarez

    Victor Perez Juarez - 2009-11-30
    • status: open-remind --> pending-remind
     

  • Carlos Ruiz

    Carlos Ruiz - 2009-11-30
    • status: pending-remind --> open-remind
     

  • Carlos Ruiz

    Carlos Ruiz - 2009-11-30

    Víctor, did you conduct tests before committing?

    Opening "Tax Rate Parent" window now throws this error in console and doesn't show any record:

    -----------> GridTable.createSelectSql: Failed to parse where clause. whereClause=(IsSummary='Y' AND C_Tax.Parent_Tax_ID=@0|C_Tax_ID@) [14]

    _________
    Carlos Ruiz

     

  • Carlos Ruiz

    Carlos Ruiz - 2009-12-07

    Reviewed thoroughly this issue.

    Kai's fix is solving some other problems in other windows - so, I think it's ok to keep it.

    Window Tax Rate Parent is having a problem because it has the same table (different records) maintained in different tabs (a circular reference). As it's seem today Adempiere is not supporting such circular references in one single window.

    I would vote to drop this window - the functionality is duplicated - Tax can be maintained without problems in "Tax Rate" window - so this "Tax Rate Parent" window is not adding anything to Adempiere - instead it's adding a window that is not working - I did a lots of test and couldn't make it work (because of the circular reference problem described) - I think Adempiere would need some deeper changes to make it work.

    My vote -> drop this window.

    What do you think?

    Regards,

    Carlos Ruiz

     

  • SCalderon

    SCalderon - 2009-12-07

    I do work with this window.
    So I am against dropping.
    Regards
    Susanne

     

  • Carlos Ruiz

    Carlos Ruiz - 2009-12-07

    Thanks Susan, I guess you work with this window in your own repository - because currently is totally broken on trunk.

    Now, the third and fourth tab are broken - so maybe it will be simple just to drop those two tabs.

    WDYT?

    BTW - if you have a working window - maybe you could share with us how is it working - because in trunk at this moment is totally useless - it's dropping a bug in console. So, you're voting to preserve which window?

    Regards,

    Carlos Ruiz

     

  • Carlos Ruiz

    Carlos Ruiz - 2009-12-08

    Committed revision 11056.
    ttp://adempiere.svn.sourceforge.net/adempiere/?rev=11056&view=rev

    Extending Kai's approach to cover timestamp and boolean variables.

    Regards,

    Carlos Ruiz

     

  • Victor Perez Juarez

    Victor Perez Juarez - 2009-12-08
    • status: open-remind --> open-works-for-me
     

  • Carlos Ruiz

    Carlos Ruiz - 2009-12-10
    • status: open-works-for-me --> pending-works-for-me
     

  • SourceForge Robot

    SourceForge Robot - 2009-12-24
    • status: pending-works-for-me --> closed-works-for-me
     

  • SourceForge Robot

    SourceForge Robot - 2009-12-24

    This Tracker item was closed automatically by the system. It was
    previously set to a Pending status, and the original submitter
    did not respond within 14 days (the time period specified by
    the administrator of this Tracker).

     

Log in to post a comment.

MongoDB Logo MongoDB