Project

General

Profile

Actions

Bug #4984

closed

Admin Ahawow backend default values of sub-parameters in arrays (e.g. CBSubs display invoice orderings) are not used correctly

Added by beat about 9 years ago. Updated almost 9 years ago.

Status:
Closed
Priority:
Urgent
Assignee:
Target version:
Start date:
07 January 2015
Due date:
% Done:

100%

Estimated time:

Description

This is due to commit 0ff7ea3b0426e023e83ffca06e8d2db30322fd74 line 555+556 which look to be a bug, as:

1. $default is not used in that get()
2. it tries to grab the index at the parent level desperately, which seems really wrong.

Assigning that to Kyle as you authorded those lines, to understand what problem they were trying to solve before removing them.

Actions #1

Updated by beat about 9 years ago

Also wondering if we should add $default parameter to the get() of line 488.

Actions #2

Updated by krileon about 9 years ago

  • Assignee changed from krileon to beat

The get() on line 560 is missing $default. Once added the issue resolves.

Adding an exception above the get() on line 560 and checking core backend views I could not find any other cases where that else would be needed. It seams to only be called when in CBSubs > Settings. It seams to have no negative affect on CBSubs > Settings when removed.

I'm not sure its purpose, but maybe it was a workaround for an issue we already resolved. I was thinking it was for repeat usage, but it seams to do nothing for repeats.. even repeats within repeats are functioning fine without it.

The most safe fix is to just add $default to that get() to avoid any possible regressions as I'm unsure what that else purpose is.

Actions #3

Updated by beat about 9 years ago

  • Status changed from Feedback to Resolved
  • % Done changed from 0 to 100

Fixed in MR !735 removing those 2 lines after Kyle's checks (Thanks Kyle), we concluded that it was initially needed for repeat fields, but that it later wasn't needed anymore after a proper fix elsewhere.

Actions #4

Updated by beat almost 9 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF