Enotices

[eileenmcnaughton]

When I go to view the contact page locally I get enotices

[colemanw]

That's really strange. Every tab is supposed to have an id.
When you call api4::ContactLayout::GetTabs() on your system, do you see any results that don't have an id?

[eileenmcnaughton]

@colemanw OK - it's tabs added by hook - https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_tabset/

• there hasn't historically been a requirement to define 'id' - before we add that to the spec it would be good to see if there is a way to not require devs to change their code.

One of the tabs comes from ExtendedReports which has a UI option to save as a tab

function extendedreport_civicrm_tabset($tabsetName, &$tabs, $context) {
  $reports = civicrm_api3('ReportInstance', 'get', array('form_values' => array('LIKE' => '%contact_dashboard_tab";s:1:"1";%')));

  foreach ($reports['values'] as $report) {
    $tabs['report_' . $report['id']] = array(
      'title' => ts($report['title']),
      'url' => CRM_Utils_System::url( 'civicrm/report/instance/' . $report['id'], array(
          'log_civicrm_address_op' => 'in',
          'contact_id_value' => $context['contact_id'],
          'contact_id' => $context['contact_id'],
          'output' => 'html',
          'force' => 1,
          'section' => 2,
          'weight' => 70,
        )
      )
    );
  }
}

[colemanw]

Tabs really need to have an ID.
I think our setting to suppress errors and notices in the smarty layer has kept you from noticing the problem.

[eileenmcnaughton]

It's not in the spec https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_tabset/

[eileenmcnaughton]

But we COULD use the key as the id if not set

[colemanw]

@eileenmcnaughton if you look at the core code, the tabs array is not associative, so the keys are meaningless.

The template is definitely expecting ids, and the only reason those hook implementations don't cause problems is that smarty suppresses php errors and browsers are quite forgiving of bad markup.

So unfortunately that documentation is wrong. We should update it and extension authors should update their code. While they're doing so would be a good time for them to add an icon to the array as well as an id.

[eileenmcnaughton]

@colemanw OK - if you update the documentation to what you think it should be I'll update extended reports & the other extension involved

[colemanw]

Ug. Ug ug ug.

I just looked closer and the documentation is correct, in a manner of speaking. The problem is that the array structure for tabsets is inconsistent. On the contact summary page you get a non-associative array with an id and the system orders them by weight. On all other tabbed pages you get an associative array with no id or weight.

Ugugugugugugugugug!

[colemanw]

Options:

1. Document the inconsistency.
2. Try to mitigate the inconsistency by being more flexible about what we accept from hooks -- @eileenmcnaughton's suggestion "use the key as the id if not set".
3. Clean up the inconsistency internally without breaking the hook contract - use one format or the other internally, but add some helper functions to fudge what we get from hooks to not break legacy hook implementations.
4. Just clean up the inconsistency internally. Make an omelette and break a few eggs.

[eileenmcnaughton]

@colemanw I would

1. update the documentation to tell people to do it the way you think they should
2. try best to handle what they might have instead in the code without too much paid - we could add a deprecated warning though - eg. in the hook if it comes back 'wrong'