APIv4 - Add Dashboard & DashboardContact entities #16867
Conversation
|
(Standard links)
|
| @@ -56,7 +56,7 @@ | |||
| </index> | |||
| <field> | |||
| <name>column_no</name> | |||
| <type>boolean</type> | |||
| <type>int</type> | |||
There was a problem hiding this comment.
I understand why they did this originally. boolean translates to tinyint in mysql (range 0-255), which I guess saves a few bytes in the database since column_number is always going to be a very small number. But now that APIv4 is stricter about field types, this gets output as true and false by api.get... which is 💩
Too bad our schema xml doesn't allow a tinyint unsigned field type, but this will do. Uses a few more bytes of disc space but whatever.
Strictly speaking this requires updating the schema sql and a database upgrade, except it doesn't matter since this change is just for the benefit of php not sql.
This field should be treated the same across all entities for consistency.
|
retest this please |
|
retest this pleaseee |
|
|
||
| // Try and find an existing dashlet - it will be updated if found. | ||
| if (!empty($params['name'])) { | ||
| if (!empty($params['name']) || !empty($params['url'])) { |
There was a problem hiding this comment.
@colemanw is this aiming to fix a bug or something, I'm not clear why we are condensing the if / else here, The code doesn't seem to be the same after
There was a problem hiding this comment.
It's essentially the same before/after. The only difference is that if both params are passed in they will both be taken into account in the search, which seems like an improvement.
| } | ||
| else { | ||
| $dashlet->id = $dashboardID; | ||
| } | ||
|
|
||
| if (isset($params['permission']) && is_array($params['permission'])) { |
There was a problem hiding this comment.
I am assuming you removed this to rely on the copyValues but the field doesn't have a serialize type in the schema so that won't happen
There was a problem hiding this comment.
ah sorry - you DID add it - I thought the above change was that DAO
| * | ||
| * @package CRM | ||
| * @copyright CiviCRM LLC https://civicrm.org/licensing | ||
| * $Id$ |
There was a problem hiding this comment.
Can we NOT add this
|
OK - this all makes sense - we should remove $ids - but that is non-blocking |
Overview
Adds APIv4 support for Dashboard & DashboardContact and does some cleanup in the BAO and API layers for improved consistency.
Before
No v4 API; older code patterns used.
After
API added, with tests; newer code patterns used.