dev/financial#65 - Revert recent changes that cause financial account edit form to be blank #15037
Conversation
|
(Standard links)
|
| @@ -53,7 +53,7 @@ public function preProcess() { | |||
| $params = [ | |||
| 'id' => $this->_id, | |||
| ]; | |||
| $financialAccount = CRM_Financial_BAO_FinancialAccount::retrieve($params); | |||
| $financialAccount = CRM_Financial_BAO_FinancialAccount::retrieve($params, CRM_Core_DAO::$_nullArray); | |||
There was a problem hiding this comment.
I think we should use api instead BAO function.
There was a problem hiding this comment.
That makes sense but it makes me realize I should put this against the RC and maybe 5.16 too since it's in 5.16. So how about just a revert for the 5.16/RC, and then switch to api for master?
The test fail is because that test was added along with same change I'm reverting, so I guess I need to do something about the test too.
There was a problem hiding this comment.
@demeritcowboy that makes sense but target the rc first (5.17) - we only backport to the current release once merged to the rc
There was a problem hiding this comment.
Does it update the target label by itself? I'm sure I've done this before and I thought it did it by itself. I rebased against upstream/5.17.
There was a problem hiding this comment.
@demeritcowboy I just updated the base branch for you (on github ui) so it should now
There was a problem hiding this comment.
Oh right. That's what I did last time. Thanks.
There was a problem hiding this comment.
@pradpnayak I tried the api but it doesn't work for the same reason that the previous change caused problems because then variables don't get set. I'm sure it's doable but it means it's not just a one-line change - I'm not really interested to do more on it.
There was a problem hiding this comment.
I'm happy with the scope of this change - in general we ask less of people when fixing regressions because a) someone is usually responding to a problem they didn't cause & b) the fix is not really optional - so for example I would also be more lax on test expectations on regressions
demeritcowboy
force-pushed
the
fin-account-limbo
branch
from
feffe0d to
7fe6eca
Compare
|
With just the |
|
unrelated fail |
Overview
The recent changes prevent the edit form from being assigned the existing values, so it just looks like a blank new form.
Before
Edit form blank.
After
Edit form has existing values.
Technical Details
Comments
https://lab.civicrm.org/dev/financial/issues/65