Closed tunbola closed 3 years ago
@tunbola @davialexandre can we also consider making the caching mechanism optional in some way? Perhaps that doesn't need to be part of this first milestone but please keep it in mind in case we do want to simplify this point.
Also - in terms of the "supported entities" vs "reports".
In future we will want to iterate to a scenario where we have a base "report templates" which define the available fields, filters and perhaps features and then an individual "report instance" which is the configured report (previously our report configurations available from the dropdown). each report instance would be listed in a report listing, then have a separate menu link to open up and view it, can be emailed automatically and also should be available for CiviCRM dashboard.
Whilst I think we support this data structure already, I think we should try and get the terminology correct now if that is possible.
@jamienovick , Will take a look at how to decouple the caching mechanism but tbh, the whole extension seems to depend on this caching mechanism as all the reports are pulled from the caching table. Will see if its possible to decouple to fetch data in real time but I wonder how effective this would be considering that even the current L&A reports fetch data from views whose data are refreshed at some intervals. Regarding the Report instance listing, I think we have the structure in place like you rightly said but will pay more attention to the terminologies and implementation.
Not needed anymore
This is not a proper PR per say but a proof of concept to demonstrate how to use the Pivot report extension to display the People reports in CiviHR.
Limitations
CRM_PivotData_DataPeople
for the Entity data class but I had to name itCRM_PivotData_DataContact
. This has some limitations, for instance it means I can't have two reports of which Contact is the main entity because there would be a name clash.civicrm_api3($EntityName, 'get', $apiParams);
See , This means that the data will always be provided via the get method of the API and additional data has to be gotten by chaining API calls together as seen here: See . This will make it difficult to get data for Entities like Leave Request that the get method can not provide all the data we need even with chaining to other API calls due to the fact that complex calculations need to be done before the data set is returned. For the People report however I was able to get it working as the data can be gotten from the API directly and by chaining API calls together.Chaining API calls has its own overhead considering the fact that the API's would be called multiple times.
Caching
civicrm_pivotreportcache
table, the cache can be built manually or automatically via a cron job. The data are are stored as Json in the data column of the table and as such is not searchable or filterable.Date Filters
Recommendations
CRM_PivotData_DataContact
we can haveCRM_PivotData_DataPeople
. The report namePeople
will be passed in the regular functions that previously expected the Entity names. The implication is that rather than the Data classes relying on thecivicrm_api3($EntityName, 'get', $apiParams);
to get the reports data, Each report class will have its owngetMethod
, exposed via the Pivot APi that will allow each report class to return data via any way it choses, either via API calls or via SQL queries. whatever method used must still support an offset and limit which is what the current API method leverages on. This way there won't be any main entity for a report and an Entity can be used across many reports to fetch information. Also each Report class will have agetCount
method that will be used everywhere and will accept filter parameters and return results based on those filters. (See the last note on limitations section). This change will be done in such a way as not to affect existing reports as the Entity name will be assumed to be the reports name.hook_civicrm_navigationMenu
, In this way, if a report is no longer valid due to a non existent dependency, its menu will not be shown on the Pivot reports menu links.civicrm_pivotreportcache
table.civicrm_api3($EntityName, 'get', $apiParams);
or the Civi APi is called directly for a report should be replaced with a call to the version of the reports method e.ggetCount
,get
exposed via the Pivot API.update on making the extension work with Leave Report
I tried to see how to get the Leave Report working with the pivot report extension as is, I came up with the following API call:
The results for some fields would still need to be modified either via handlers or in the logic when results are fetched e.g Employee Age group(by modifying the birth_date field), Employee Age, Absence Is Credit(by modifying the request_type field), Absence Day of Week, Absence Start Month, Absence End month.... The API calls to
api.HRJobPay.get
andapi.HRJobHour.get
did not return any results, This seems to be because thejobcontract_revision_id
field returned by the Job contract API is not a true field of the Entity per say because the field actually belongs to the HRJobDetails Entity.Update on other extensions injecting their report.
I tried making the People report to reside in L&A extension and it was successful to some extent.
CRM_PivotCache_GroupContact and
CRM_PivotData_DataContactbut I could not add them in the
HRLeaveAndAbsences` namespace but outside it due to this, i.e the namespace for the extending classes are already hardcoded.