First batch of code review changes

- Refactor function signature for getApiPath
= Update usages and unit tests

Rework getApiPath to use URLSearchParams

* Refactor and add unit tests

Favor using vue slots instead of prop with conditional

* Refactor implementation

Show label in Security tab

* Show label in security tab

app/assets/javascripts/api.js

@@ -782,37 +782,44 @@ const Api = {
    *
    * Example:
    *
-   * User provides string from
+   * API: https://docs.gitlab.com/ee/api/job_artifacts.html#download-the-artifacts-archive
    *
-   * expose_path(api_v4_projects_pipelines_jobs_fuzing_artifacts_path(id: project.id, job_ref: ':job_ref'))
+   * User provides string to the frontend from:
+   *
+   * expose_path(api_v4_projects_pipelines_jobs_artifacts_path(id: project.id, ref_name: ':ref_name'))
+   *
+   * Rails route helper generated string:
+   *
+   * https://gitlab.example.com/api/v4/projects/1234/jobs/artifacts/:ref_name/download
    *
    * User calls:
    *
-   *  getApiPath(this.fuzzingArtifactsPath, {
-   *   keys: { ':job_ref': job.ref },
-   *   params: {job: job.name}
+   *  getApiPath(this.artifactsPath, {
+   *   params { ':ref_name': 'master' },
+   *   query: {job: 'job-name'}
    *  }
    *
    * Output is:
    *
-   * https://gitlab.com/api/v4/projects/19413496/jobs/artifacts/refs/merge-requests/5/head/download?job=my_fuzz_target
+   * https://gitlab.example.com/api/v4/projects/1234/jobs/artifacts/master/download?job=job-name
    *
    * @param {String} path The stubbed url
-   * @param {Object} keys The stubbed values to replace
-   * @param {Object} params The url query params
+   * @param {Object} params The stubbed values to replace
+   * @param {Object} query The url query params
    * @returns {String} The dynamically generated URL
    */
-  getApiPath(path = '', { keys, params }) {
+  getApiPath(path = '', { params = {}, query = {} }) {
     if (path) {
       let outputPath = path;
-      Object.entries(keys).forEach(([key, value]) => {
+      const outputQuery = new URLSearchParams(query);
+
+      // Replace the :ruby_symbols
+      Object.entries(params).forEach(([key, value]) => {
         outputPath = outputPath.replace(key, value);
       });
-      return params
-        ? `${outputPath}?${Object.keys(params)
-            .map(key => `${key}=${params[key]}`)
-            .join('&')}`
-        : outputPath;
+
+      // Construct the url query params
+      return Object.keys(query).length ? `${outputPath}?${outputQuery}` : outputPath;
     }
     return path;
   },

ee/app/assets/javascripts/security_dashboard/components/fuzzing_artifacts_download.vue

@@ -20,11 +20,6 @@ export default {
       type: Number,
       required: true,
     },
-    hasLabel: {
-      type: Boolean,
-      required: false,
-      default: true,
-    },
   },
   computed: {
     hasDropdown() {
@@ -43,7 +38,7 @@ export default {
 
 <template>
   <div>
-    <strong v-if="hasLabel">{{ s__('SecurityReports|Download Report') }}</strong>
+    <slot name="label"></slot>
     <gl-dropdown
       v-if="hasDropdown"
       class="d-block mt-1"

ee/app/assets/javascripts/security_dashboard/components/security_dashboard_vuex.vue

@@ -167,7 +167,11 @@ export default {
           <vulnerability-count-list v-if="shouldShowCountList" />
           <filters>
             <template v-if="hasFuzzingArtifacts" #buttons>
-              <fuzzing-artifacts-download :jobs="fuzzingJobsWithArtifact" :project-id="projectId" />
+              <fuzzing-artifacts-download :jobs="fuzzingJobsWithArtifact" :project-id="projectId">
+                <template #label>
+                  <strong>{{ s__('SecurityReports|Download Report') }}</strong>
+                </template>
+              </fuzzing-artifacts-download>
             </template>
           </filters>
         </template>

ee/app/assets/javascripts/vue_shared/security_reports/grouped_security_reports_app.vue

@@ -317,7 +317,7 @@ export default {
 
     const coverageFuzzingDiffEndpoint = gl?.mrWidgetData?.coverage_fuzzing_comparison_path;
     const pipelineJobsPath = Api.getApiPath(gl?.mrWidgetData?.pipeline_jobs_path, {
-      keys: { ':pipeline_id': gl?.mrWidgetData?.pipeline_id },
+      params: { ':pipeline_id': gl?.mrWidgetData?.pipeline_id },
     });
 
     if (coverageFuzzingDiffEndpoint && this.hasCoverageFuzzingReports) {
@@ -571,7 +571,6 @@ export default {
               v-if="hasFuzzingArtifacts"
               :jobs="fuzzingJobsWithArtifact"
               :project-id="projectId"
-              :has-label="false"
             />
           </summary-row>
 

ee/spec/frontend/api_spec.js

@@ -870,7 +870,6 @@ describe('Api', () => {
     });
   });
 
-<<<<<<< HEAD
   describe('Billable members list', () => {
     let expectedUrl;
     let namespaceId;
@@ -887,20 +886,24 @@ describe('Api', () => {
         return Api.fetchBillableGroupMembersList(namespaceId).then(({ data }) => {
           expect(data).toEqual([]);
         });
-=======
+      });
+    });
+  });
+
   describe('getApiPath', () => {
     describe.each`
-      path                           | keys                                   | params                           | expected
+      path                           | params                                 | query                            | expected
       ${undefined}                   | ${{ ':user_id': 4 }}                   | ${{ project_id: 3 }}             | ${''}
       ${'/foo/bar'}                  | ${{ ':user_id': 4 }}                   | ${{ project_id: 3 }}             | ${'/foo/bar?project_id=3'}
       ${'/:user_id/bar'}             | ${{ ':user_id': 4 }}                   | ${{ project_id: 3 }}             | ${'/4/bar?project_id=3'}
       ${'/:wrong_id/bar'}            | ${{ ':user_id': 4 }}                   | ${{ project_id: 3 }}             | ${'/:wrong_id/bar?project_id=3'}
       ${'/:first_id/bar/:second_id'} | ${{ ':first_id': 1, ':second_id': 2 }} | ${{ project_id: 3 }}             | ${'/1/bar/2?project_id=3'}
       ${'/:first_id/bar/:second_id'} | ${{ ':first_id': 1, ':second_id': 2 }} | ${{ project_id: 3, user_id: 7 }} | ${'/1/bar/2?project_id=3&user_id=7'}
-    `('When path: $path, keys: $keys, params: $params', ({ path, keys, params, expected }) => {
+      ${'/:first_id/bar/:second_id'} | ${undefined}                           | ${{ project_id: 3, user_id: 7 }} | ${'/:first_id/bar/:second_id?project_id=3&user_id=7'}
+      ${'/:first_id/bar/:second_id'} | ${{ ':first_id': 1, ':second_id': 2 }} | ${undefined}                     | ${'/1/bar/2'}
+    `('When path: $path, params: $params, query: $query', ({ path, params, query, expected }) => {
       it(`returns ${expected}`, () => {
-        expect(Api.getApiPath(path, { keys, params })).toBe(expected);
->>>>>>> 55722995fad... Add unit tests for api helper
+        expect(Api.getApiPath(path, { params, query })).toBe(expected);
       });
     });
   });