Skip to content

[MR Widget Eng] - Make single endpoint polling consistent with multi-poll functionality.

This is a follow up to !87107 (merged)

That MR does the following:

  • Allows the user to pass in an array of endpoints to poll
  • Handles 204-No Content gracefully by continuing to poll if POLL-INTERVAL header is set.

What we need to do:

Replicate the logic from !87107 (merged) and initExtensionMultiPolling and have it behave the same as initExtensionPolling() in regards to checking the poll interval headers.

Here is a proposed patch:

We still need to update the logic in initExtensionPolling to support a success status code where data is undefined. When we get a 204 - No Content on the initial poll, data is undefined

Object.keys(data) causes an exception. We need to update initExtensionPolling to also expect the full response object so we can also do perhaps the following patch:

Note

With this patch we would need to update existing widgets with enablePolling to pass the full response.

It just feels odd to handle the headers in the multi-poll scenario and not the single endpoint scenario. I feel like the behavior should be consistent. Open to your thoughts.

diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue
index bdb277843599..cc221674f6a0 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue
@@ -159,12 +159,7 @@ export default {
           },
           method: 'fetchData',
           successCallback: (response) => {
-            const headers = normalizeHeaders(response.headers);
-
-            if (typeof headers['POLL-INTERVAL'] === 'undefined') {
-              poll.stop();
-              allData.push(response.data);
-            }
+            this.headerCheck(poll, response, allData.push)
 
             if (allData.length === requests.length) {
               this.setCollapsedData(allData);
@@ -185,11 +180,8 @@ export default {
           fetchData: () => this.fetchCollapsedData(this.$props),
         },
         method: 'fetchData',
-        successCallback: ({ data }) => {
-          if (Object.keys(data).length > 0) {
-            poll.stop();
-            this.setCollapsedData(data);
-          }
+        successCallback: (response) => {
+          this.headerCheck(poll, response, this.setCollapsedData)
         },
         errorCallback: (e) => {
           poll.stop();
@@ -200,6 +192,14 @@ export default {
 
       poll.makeRequest();
     },
+    headerCheck(poll, response, func){
+      const headers = normalizeHeaders(response.headers);
+
+      if (typeof headers['POLL-INTERVAL'] === 'undefined') {
+        poll.stop();
+        func(response.data);
+      }
+    },
     loadCollapsedData() {
       this.loadingState = LOADING_STATES.collapsedLoading;

204.patch

  1. Update existing components with enablePolling: true behave correctly with the changes in the implementation plan by passing the full response object instead of the data only. Update unit tests as well

app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js

app/assets/javascripts/vue_merge_request_widget/extensions/test_report/index.js

ee/app/assets/javascripts/vue_merge_request_widget/extensions/metrics/index.js

Edited by -