Skip to content

Conversation

@ShadMickelberry
Copy link

@ShadMickelberry ShadMickelberry commented Feb 8, 2018

This is a first step for a Issue #20 to create a Sucrose chart dashlet to track changes. The chart in mind would track changes in Student health status across time. To do this we to track the changes on this field to query the audit table.

@mmarum-sugarcrm
Copy link
Member

Thanks @ShadMickelberry for creating this PR! I'm wondering if you could elaborate on the story of how you see Professor M managing student health over time.

@ShadMickelberry
Copy link
Author

ShadMickelberry commented Feb 13, 2018

Hello @mmarum-sugarcrm. The idea I had was to have a horizontal multi-bar dashlet. The x-axis would be time (dropdown in config would allow variable timeframes. The y-axis would be students. And the data would be changes in student vitals calculated in days.

For example, if a student was Active the whole time their bar would be all one color or 365 for 1 yr. However, if they had moved to Injured the bar would change to a different color and change again if they went back to Active or something else.

Another way would be to create a status changes module that would track the changes in Vitals instead of storing that data in the contacts_audit table.

I am not great with unit testing yet but am trying to learn. I couldn't think of a way to test the audit record being created when I tried over the weekend.

I'm definitely open to suggestions or change of scope.

@mmarum-sugarcrm
Copy link
Member

@ShadMickelberry Are you coming to SKO? I'll be there next week. I agree that to display historical data we need to make sure the field is audited. I need to think about the APIs we'd use to fetch this historical info and display it in a dashlet. I think you can only get audit log on one record at a time via REST API. I need to test that out.

Building automated tests for this would definitely require some sort of integration test. Assuming that Sugar's core audit log works properly, you'd want to be testing that the dashlet processes API responses correctly. A unit test that verifies that "audit" is true for this key field would probably be sufficient for back-end testing.

@ShadMickelberry
Copy link
Author

@mmarum-sugarcrm Unfortunately I will not be making it to SKO this year.
I had in mind a custom API endpoint the would do the necessary queries to poll the audit logs. It would be too costly to do multiple queries to get the audit data individually. We also only want the vitals_c changes and I'm not sure if that flexibility is available.

@ShadMickelberry
Copy link
Author

@mmarum-sugarcrm Added the dashlet files and API to retrieve data for a sucrose pie chart. I started adding php-unit tests using the legacy framework but wasn't sure if that was going to be used in this project or not. I would like to add some Jasmine tests too but just started working on those.

@mmarum-sugarcrm
Copy link
Member

Thanks Shad. Sorry for delay, we will review this PR soon.

Copy link
Member

@mmarum-sugarcrm mmarum-sugarcrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Shad. Could you address requested changes and test this with Sugar 8? I'm seeing problems with the custom REST endpoint when running it on Sugar 8.


global $app_list_strings;
require_once('custom/include/ProfessorM/StudentVitalHelper.php');
$team = $args['team'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use a different name for this argument throughout the code so that nobody confuse it with Sugar's Teams functionality. It's really the supergroup id, correct?

'fields' => array(

array(
'name' => 'vitals_dashlet_team',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default width of this enum field is too narrow. It should probably be resized after fetching list of options. Also, lets avoid "team" term and use something like supergroup instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the corresponding js controller so column width is preserved when populating data dynamically

@ShadMickelberry
Copy link
Author

Sorry for the long delay. I'm getting back on this and should have some updates soon.

@mmarum-sugarcrm
Copy link
Member

@ShadMickelberry Hey - I simplified this PR a bit (cut the scope back) and got it working with latest Sugar versions. Can you check out PR #127 and let me know what you think?

@ShadMickelberry
Copy link
Author

@mmarum-sugarcrm Thanks for the feedback and input on this. I think cutting the scope makes a great deal of sense. This Issue ended up having more to it than I originally thought and could have been better organized by me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants