Display API monitoring history #3533

vinaytech · 2018-07-26T10:50:10Z

Closes #3521

Changes

Describe your changes here as a bulleted list:

change in file monitoring collection and permission to remove the bug
change now save date in response object

Developer checklist

This checklist is to be completed by the PR developer:

Alternative solutions were compared/discussed before writing code
- trade-offs with this solution are considered acceptable
Code in this PR adheres to the project styleguide
This pull request does not decrease project test coverage
If the code changes existing database collection(s), migration has been written
If UI texts are added or changed, all texts are internationalized

Reviewer checklist

Reviewed by: @55

This list is to be completed by the pull request reviewer:

55

Solution is incomplete. Table is missing. Please check this #3521 (comment).

55 · 2018-08-01T11:25:34Z

@matleppa review the code, I will do the layout.

matleppa

Some changes needed.
In addition to that, I commented the issue 3521 about inputting here only (API base path and) the endpoint. And about displaying the endpoint in monitoring table.

matleppa · 2018-08-01T12:11:05Z

apinf_packages/apis/server/methods/status.js

+Meteor.publish('getApiStatuRecordsData', (apiId) => {
+  // Make sure apiId is a string
+  check(apiId, String);
+  // Find all API Backends


Change comment to be:
Find monitoring data from previous 24 hours

matleppa · 2018-08-01T12:12:15Z

apinf_packages/apis/server/methods/status.js

+  // Find all API Backends
+  const startDate = new Date();
+  const lastDate = new Date();
+  lastDate.setDate(lastDate.getDate() - 2);


To get only last 24 hours
--> lastDate.setDate(lastDate.getDate() - 1);

matleppa · 2018-08-01T12:12:44Z

apinf_packages/apis/server/methods/status.js

@@ -11,6 +11,17 @@ import { check } from 'meteor/check';
 import Apis from '/apinf_packages/apis/collection';
 import { MonitoringData } from '/apinf_packages/monitoring/collection';

+Meteor.publish('getApiStatuRecordsData', (apiId) => {


'getApiStatuRecordsData'
-->
'getApiStatusRecordData'

matleppa · 2018-08-01T12:17:01Z

apinf_packages/core/lib/i18n/en.i18n.json

@@ -145,6 +145,7 @@
  "apiIntro_steps_settings_intro": "Edit API settings from this tab. You can also delete API here.",
  "apiIntro_steps_welcome_intro": "Welcome",
  "apiIntro_skipLabel": "Skip",
+  "apiIntro_steps_monitoring_data": "Monitoring data",


--> "Start and stop API monitoring. When monitoring is on, the response statuses during latest 24 hours are displayed."

Replace value "Monitoring data" with text "Start and stop API monitoring. When monitoring is on, the response statuses during latest 24 hours are displayed."

matleppa · 2018-08-01T12:18:51Z

apinf_packages/monitoring/client/monitoring.html

-
-        {{> afQuickField name='enabled' }}
+  {{# if apiMonitoringSettings.enabled }}
+    <h2 style="font-size:1.4em;margin-top:0;">


Use style classes instead of direct style in element.

matleppa · 2018-08-01T12:19:03Z

apinf_packages/monitoring/client/monitoring.html

+    <h2 style="font-size:1.4em;margin-top:0;">
+      Status Messages
+    </h2>
+    <p style="color:#6d859e;">


Use style classes instead of direct style in element.

matleppa · 2018-08-01T12:19:45Z

apinf_packages/monitoring/client/monitoring.html

+      </tbody>
+    </table>
+
+    <hr style="color: rgba(59, 59, 88, 0.15); margin-right: -20px; margin-left: -20px; margin-top: 2em; margin-bottom: 2em;">


Use style classes instead of direct style in element.

matleppa · 2018-08-01T12:21:23Z

apinf_packages/monitoring/client/monitoring.html

+  <h3 style="font-size:16px; margin-top:0;">
+    {{_ "apiMonitoring_panelTitle_Monitoring" }}
+  </h3>
+  <p style="color:#6d859e;">{{_ "apiMonitoring_helpIcon_text" }}</p>


Use style classes instead of direct style in element.

matleppa · 2018-08-01T12:23:33Z

apinf_packages/monitoring/client/monitoring.js

@@ -18,8 +18,8 @@ Template.apiMonitoring.onCreated(function () {

  // Subscribe on Monitoring collection
  instance.subscribe('monitoringSettings', apiId);
+  instance.subscribe('getApiStatuRecordsData', apiId);


'getApiStatuRecordsData'
-->
'getApiStatusRecordData'

matleppa · 2018-08-01T13:00:04Z

apinf_packages/monitoring/collection/permissions.js

+
+MonitoringData.allow({
+  insert () {
+    // Get API document


Remove unnecessary comment

matleppa · 2018-08-02T07:01:40Z

apinf_packages/monitoring/client/monitoring.js

+    // Get API ID
+    const apiId = this.api._id;
+    const monitoringDatas = MonitoringData.findOne({ apiId });
+    return monitoringDatas.responses;


Add checking and return monitoringDatas.responses only in case monitoringData was found.

matleppa · 2018-08-02T11:19:40Z

apinf_packages/apis/client/profile/view.html

@@ -28,6 +28,10 @@
          {{> apiFeedback api=api feedbackItems=feedbackItems }}
        </div> <!-- /api-feedback -->

+        <div id="api-monitoring" class="tab-pane fade">
+         {{> apiMonitoring api=api }}


Two space indentation.

matleppa · 2018-08-02T11:21:06Z

apinf_packages/monitoring/client/monitoring.html

+      Historical data on API availability
+    </p>
+    <table id="rtable">
+        <tr>


Correct indentations to two spaces.

matleppa · 2018-08-02T11:22:04Z

apinf_packages/monitoring/client/monitoring.html

+        <label for="endpoint-monitor-field">
+          {{ afFieldLabelText name='url' }}
+        </label>
+          {{> afFieldInput name='url' }}


Remove unnecessary indentation.

Unnecessary indentation for the input field.

matleppa · 2018-08-02T11:22:50Z

apinf_packages/monitoring/client/monitoring.less

+@import "/apinf_packages/core/client/style/bootstrap-variables.less";
+
+#rtable {
+    font-family: "Trebuchet MS", Arial, Helvetica, sans-serif;


Correct indentations to two spaces.

…d with data = false is found, create a record for API in question into MonitoringData and fill the id of created record into field data in MonitoringSettings.

matleppa

The functionality seems to be correct. I also verified, that the migration works as expected.
Good work!

There still are a few minor findings. After you have corrected them, I'll approve this PR.

In original issue there are comments about displaying also the endpoint, against what the API status is checked.
I'll make another issue about them, so we get this one closed.

matleppa · 2018-08-03T12:34:06Z

apinf_packages/apis/server/methods/status.js

@@ -11,6 +11,17 @@ import { check } from 'meteor/check';
 import Apis from '/apinf_packages/apis/collection';
 import { MonitoringData } from '/apinf_packages/monitoring/collection';

+Meteor.publish('getApiStatuRecordData', (apiId) => {


getApiStatuRecordData
-->
getApiStatusRecordData

matleppa · 2018-08-03T12:35:12Z

apinf_packages/core/lib/i18n/en.i18n.json

@@ -145,6 +145,7 @@
  "apiIntro_steps_settings_intro": "Edit API settings from this tab. You can also delete API here.",
  "apiIntro_steps_welcome_intro": "Welcome",
  "apiIntro_skipLabel": "Skip",
+  "apiIntro_steps_monitoring_data": "Monitoring data",


Replace value "Monitoring data" with text "Start and stop API monitoring. When monitoring is on, the response statuses during latest 24 hours are displayed."

matleppa · 2018-08-03T12:35:56Z

apinf_packages/monitoring/client/monitoring.html

+        <label for="endpoint-monitor-field">
+          {{ afFieldLabelText name='url' }}
+        </label>
+          {{> afFieldInput name='url' }}


Unnecessary indentation for the input field.

matleppa · 2018-08-03T12:36:13Z

apinf_packages/monitoring/client/monitoring.js

@@ -18,8 +18,8 @@ Template.apiMonitoring.onCreated(function () {

  // Subscribe on Monitoring collection
  instance.subscribe('monitoringSettings', apiId);
+  instance.subscribe('getApiStatuRecordData', apiId);


getApiStatuRecordData
-->
getApiStatusRecordData

…g of function name

matleppa

Functionality as expected.

matleppa · 2018-08-06T12:44:10Z

@55 Please have a look at this PR 3533. If your change requests are met, please approve them so I can merge the PR.
I have created another issue about monitoring endpoint changes (input & display), so they are outside of scope of this PR.

vinaytech added 5 commits July 20, 2018 14:33

Monitoring history

9dbcb27

Merge branch 'develop' into apinf/monitoring-history-3521

c8356ab

changes for monitoring history feature

110494b

changes for monitoring history feature

87ea243

changes for Monitoring history 3521

c8ef5d0

ghost assigned vinaytech Jul 26, 2018

ghost added the in progress label Jul 26, 2018

vinaytech removed their assignment Jul 26, 2018

55 self-requested a review July 26, 2018 11:02

vinaytech added Ready for review and removed in progress labels Jul 27, 2018

55 suggested changes Jul 27, 2018

View reviewed changes

55 added in progress and removed Ready for review labels Jul 27, 2018

apply Monitoring UI

8fc899d

ghost assigned vinaytech Jul 31, 2018

vinaytech added 3 commits July 31, 2018 13:04

apply Monitoring UI

f2e9b94

Monitoring api status apply with new tab

d02b90d

changes for monitoring tab position

65031b5

vinaytech added Ready for review and removed in progress labels Aug 1, 2018

55 requested a review from matleppa August 1, 2018 11:25

matleppa requested changes Aug 1, 2018

View reviewed changes

matleppa reviewed Aug 2, 2018

View reviewed changes

55 added in progress and removed Ready for review labels Aug 2, 2018

matleppa added Ready for review and removed Ready for review labels Aug 2, 2018

changes to remove inline css to less file

961f678

matleppa requested changes Aug 2, 2018

View reviewed changes

vinaytech added 2 commits August 2, 2018 17:40

changes for code format remove extra space

151a018

show response only 24 hours

83a9381

vinaytech added Ready for review and removed in progress labels Aug 2, 2018

add migration to loop through MonitoringSettings table and if a recor…

d63988b

…d with data = false is found, create a record for API in question into MonitoringData and fill the id of created record into field data in MonitoringSettings.

matleppa requested changes Aug 3, 2018

View reviewed changes

matleppa added in progress and removed Ready for review labels Aug 3, 2018

changes for replace Monitoring date with given one and change spellin…

e16894b

…g of function name

vinaytech added Ready for review and removed in progress labels Aug 6, 2018

matleppa approved these changes Aug 6, 2018

View reviewed changes

matleppa changed the title ~~Apinf/monitoring history 3521~~ Display API monitoring history Aug 6, 2018

55 approved these changes Aug 7, 2018

View reviewed changes

55 merged commit 21329cc into develop Aug 7, 2018

ghost removed the Ready for review label Aug 7, 2018

55 deleted the apinf/monitoring-history-3521 branch August 7, 2018 08:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display API monitoring history #3533

Display API monitoring history #3533

vinaytech commented Jul 26, 2018 •

edited by 55

Loading

55 left a comment •

edited

Loading

55 commented Aug 1, 2018

matleppa left a comment

matleppa Aug 1, 2018

matleppa Aug 1, 2018

matleppa Aug 1, 2018

matleppa Aug 1, 2018

matleppa Aug 3, 2018

matleppa Aug 1, 2018

matleppa Aug 1, 2018

matleppa Aug 1, 2018

matleppa Aug 1, 2018

matleppa Aug 1, 2018

matleppa Aug 1, 2018

matleppa Aug 2, 2018

matleppa Aug 2, 2018

matleppa Aug 2, 2018

matleppa Aug 2, 2018

matleppa Aug 3, 2018 •

edited

Loading

matleppa Aug 2, 2018

matleppa left a comment

matleppa Aug 3, 2018

matleppa Aug 3, 2018

matleppa Aug 3, 2018 •

edited

Loading

matleppa Aug 3, 2018

matleppa left a comment

matleppa commented Aug 6, 2018

Display API monitoring history #3533

Display API monitoring history #3533

Conversation

vinaytech commented Jul 26, 2018 • edited by 55 Loading

Changes

Developer checklist

Reviewer checklist

55 left a comment • edited Loading

Choose a reason for hiding this comment

55 commented Aug 1, 2018

matleppa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matleppa Aug 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matleppa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matleppa Aug 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matleppa left a comment

Choose a reason for hiding this comment

matleppa commented Aug 6, 2018

vinaytech commented Jul 26, 2018 •

edited by 55

Loading

55 left a comment •

edited

Loading

matleppa Aug 3, 2018 •

edited

Loading

matleppa Aug 3, 2018 •

edited

Loading