From c4ec9866f663b429442c5f50407e04cd59e718df Mon Sep 17 00:00:00 2001 From: Katarina Zailac Date: Wed, 12 Jun 2024 15:29:44 +0200 Subject: [PATCH 01/11] Include profile information in metrics GET method --- poem/Poem/api/internal_views/metrics.py | 7 +++++-- poem/Poem/api/tests/test_metrics.py | 19 +++++++++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/poem/Poem/api/internal_views/metrics.py b/poem/Poem/api/internal_views/metrics.py index 2074b8148..2aadce9c6 100644 --- a/poem/Poem/api/internal_views/metrics.py +++ b/poem/Poem/api/internal_views/metrics.py @@ -46,11 +46,12 @@ def get(self, request, name=None): if name: metrics = poem_models.Metric.objects.filter(name=name) if metrics.count() == 0: - raise NotFound(status=404, - detail='Metric not found') + raise NotFound(status=404, detail='Metric not found') else: metrics = poem_models.Metric.objects.all() + profiles4metrics = get_metrics_in_profiles(request.tenant) + results = [] for metric in metrics: if metric.probeversion: @@ -88,6 +89,8 @@ def get(self, request, name=None): name=metric.name, mtype=mt.mtype.name, tags=[tag.name for tag in mt.tags.all()], + profiles=profiles4metrics[metric.name] + if metric.name in profiles4metrics else [], probeversion=metric.probeversion if metric.probeversion else "", group=group, description=mt.description, diff --git a/poem/Poem/api/tests/test_metrics.py b/poem/Poem/api/tests/test_metrics.py index 9fef91a86..bd9843534 100644 --- a/poem/Poem/api/tests/test_metrics.py +++ b/poem/Poem/api/tests/test_metrics.py @@ -313,6 +313,11 @@ def mock_db(): content_type=ct ) +profiles4metrics = { + "argo.AMS-Check": ["ARGO_MON", "ARGO_MON_CRITICAL"], + "argo.AMSPublisher-Check": ["ARGO_MON_INTERNAL"] +} + class ListAllMetricsAPIViewTests(TenantTestCase): @factory.django.mute_signals(pre_save) @@ -424,8 +429,11 @@ def setUp(self): name="org.apel.APEL-Pub" ) - def test_get_metric_list(self): + @patch("Poem.api.internal_views.metrics.get_metrics_in_profiles") + def test_get_metric_list(self, mock_profiles4metrics): + mock_profiles4metrics.return_value = profiles4metrics request = self.factory.get(self.url) + request.tenant = self.tenant force_authenticate(request, user=self.user) response = self.view(request) self.assertEqual( @@ -436,6 +444,7 @@ def test_get_metric_list(self): 'name': 'argo.AMS-Check', 'mtype': 'Active', 'tags': ['test_tag1', 'test_tag2'], + "profiles": ["ARGO_MON", "ARGO_MON_CRITICAL"], 'probeversion': 'ams-probe (0.1.7)', 'group': 'EGI', 'description': 'Description of argo.AMS-Check', @@ -490,6 +499,7 @@ def test_get_metric_list(self): 'name': 'argo.AMSPublisher-Check', 'mtype': 'Active', 'tags': ['test_tag1'], + "profiles": ["ARGO_MON_INTERNAL"], 'probeversion': 'ams-publisher-probe (0.1.7)', 'group': 'EUDAT', 'description': '', @@ -538,6 +548,7 @@ def test_get_metric_list(self): 'name': 'org.apel.APEL-Pub', 'mtype': 'Passive', 'tags': [], + "profiles": [], 'probeversion': '', 'group': 'EGI', 'description': '', @@ -563,8 +574,11 @@ def test_get_metric_list(self): ] ) - def test_get_metric_by_name(self): + @patch("Poem.api.internal_views.metrics.get_metrics_in_profiles") + def test_get_metric_by_name(self, mock_profiles4metrics): + mock_profiles4metrics.return_value = profiles4metrics request = self.factory.get(self.url + 'argo.AMS-Check') + request.tenant = self.tenant force_authenticate(request, user=self.user) response = self.view(request, 'argo.AMS-Check') self.assertEqual( @@ -574,6 +588,7 @@ def test_get_metric_by_name(self): 'name': 'argo.AMS-Check', 'mtype': 'Active', 'tags': ['test_tag1', 'test_tag2'], + "profiles": ["ARGO_MON", "ARGO_MON_CRITICAL"], 'probeversion': 'ams-probe (0.1.7)', 'group': 'EGI', 'description': 'Description of argo.AMS-Check', From 81fd7e3049cf37bcf63e2eed2cdf0be7469864e2 Mon Sep 17 00:00:00 2001 From: Katarina Zailac Date: Thu, 13 Jun 2024 08:26:31 +0200 Subject: [PATCH 02/11] New message when deleting metrics --- poem/Poem/frontend/react/Metrics.js | 30 ++++++++++++++----- .../frontend/react/__tests__/Metrics.test.js | 5 ++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/poem/Poem/frontend/react/Metrics.js b/poem/Poem/frontend/react/Metrics.js index 6ace548c4..7be3234f1 100644 --- a/poem/Poem/frontend/react/Metrics.js +++ b/poem/Poem/frontend/react/Metrics.js @@ -860,6 +860,7 @@ export const MetricForm = defaultValues: { id: initValues.id ? initValues.id : "", name: initValues.name, + profiles: initValues.profiles, probeversion: initValues.probeversion, type: initValues.type, group: initValues.group ? initValues.group: "", @@ -968,6 +969,25 @@ export const MetricForm = toggleAreYouSure() } + function onDeleteHandle() { + let profiles = getValues("profiles") + let name = getValues("name") + let msg = "" + + if (resourcename === "metric" && !isHistory && profiles.length > 0) { + msg =
+

Metric { name } is part of profile(s): { profiles.join(", ") }

+

ARE YOU SURE you want to delete it?

+
+ } else + msg = `Are you sure you want to delete ${resourcename_beautify}?` + + setModalMsg(msg) + setModalTitle(`Delete ${resourcename_beautify}`) + setModalFlag('delete') + toggleAreYouSure() + } + return ( { - setModalMsg(`Are you sure you want to delete ${resourcename_beautify}?`) - setModalTitle(`Delete ${resourcename_beautify}`) - setModalFlag('delete') - toggleAreYouSure() - }} + onClick={() => onDeleteHandle()} > Delete @@ -1707,7 +1722,8 @@ export const MetricChange = (props) => { file_attributes: metric.files, file_parameters: metric.fileparameter, probe: probe, - tags: metric.tags + tags: metric.tags, + profiles: metric.profiles }} isTenantSchema={ true } groups={ groups } diff --git a/poem/Poem/frontend/react/__tests__/Metrics.test.js b/poem/Poem/frontend/react/__tests__/Metrics.test.js index 65f91871c..08768e7f1 100644 --- a/poem/Poem/frontend/react/__tests__/Metrics.test.js +++ b/poem/Poem/frontend/react/__tests__/Metrics.test.js @@ -15,6 +15,7 @@ const mockListOfMetrics = [ name: 'argo.AMS-Check', mtype: 'Active', tags: ['test_tag1', 'test_tag2'], + profiles: ["ARGO_MON", "TEST_PROFILE"], probeversion: 'ams-probe (0.1.12)', group: 'EGI', description: 'Description of argo.AMS-Check metric', @@ -44,6 +45,7 @@ const mockListOfMetrics = [ name: 'argo.AMS-Publisher', mtype: 'Active', tags: ['internal'], + profiles: ["ARGO_MON_INTERNAL"], probeversion: 'ams-publisher-probe (0.1.12)', group: 'EGI', description: '', @@ -74,6 +76,7 @@ const mockListOfMetrics = [ name: 'org.apel.APEL-Pub', mtype: 'Passive', tags: [], + profiles: [], probeversion: '', group: 'ARGOTEST', description: '', @@ -97,6 +100,7 @@ const mockMetric = { name: 'argo.AMS-Check', mtype: 'Active', tags: ['test_tag1', 'test_tag2'], + profiles: ["ARGO_MON", "TEST_PROFILE"], probeversion: 'ams-probe (0.1.12)', group: 'EGI', description: 'Description of argo.AMS-Check metric', @@ -127,6 +131,7 @@ const mockPassiveMetric = { name: 'org.apel.APEL-Pub', mtype: 'Passive', tags: [], + profiles: [], probeversion: '', group: 'ARGOTEST', description: '', From 1cc7a015aed7abf7dede4be152d3e43d88001a88 Mon Sep 17 00:00:00 2001 From: Katarina Zailac Date: Thu, 13 Jun 2024 10:45:27 +0200 Subject: [PATCH 03/11] Move metric templates table to a separate function --- poem/Poem/frontend/react/MetricTags.js | 365 +++++++++++++------------ 1 file changed, 195 insertions(+), 170 deletions(-) diff --git a/poem/Poem/frontend/react/MetricTags.js b/poem/Poem/frontend/react/MetricTags.js index ad586fd93..c69b9f754 100644 --- a/poem/Poem/frontend/react/MetricTags.js +++ b/poem/Poem/frontend/react/MetricTags.js @@ -1,4 +1,4 @@ -import React, { useState } from "react" +import React, { useContext, useState } from "react" import { useMutation, useQuery, useQueryClient } from "react-query" import { Link, useLocation, useParams, useNavigate } from "react-router-dom" import { fetchMetricTags, fetchMetricTemplates, fetchUserDetails } from "./QueryFunctions" @@ -31,7 +31,7 @@ import { } from "reactstrap" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome" import { faSearch,faPlus, faTimes } from '@fortawesome/free-solid-svg-icons'; -import { Controller, useForm, useWatch } from "react-hook-form" +import { Controller, FormProvider, useForm, useFormContext, useWatch } from "react-hook-form" import { ErrorMessage } from '@hookform/error-message' import { yupResolver } from '@hookform/resolvers/yup' import * as Yup from "yup" @@ -46,6 +46,9 @@ const validationSchema = Yup.object().shape({ }) +const MetricTagsContext = React.createContext() + + export const MetricTagsList = (props) => { const location = useLocation(); const publicView = props.publicView @@ -120,6 +123,125 @@ export const MetricTagsList = (props) => { } +const MetricsList = () => { + const context = useContext(MetricTagsContext) + + const { control, getValues, setValue } = useFormContext() + + return ( + + + + + + { + !context.publicView && + } + + + + + + + + { + context.metrics4tag.filter( + filteredRow => filteredRow.toLowerCase().includes(context.searchItem.toLowerCase()) + ).map((item, index) => + + + + + { + !context.publicView && + + } + + + ) + } + +
#Metric templateActions
+ + + + + } + /> +
+ { index + 1 } + + { + context.publicView ? + item + : + + { + let tmpMetrics = getValues("metrics4tag") + tmpMetrics[index] = e.value + setValue("metrics4tag", tmpMetrics) + } } + options={ + context.allMetrics.map( + met => met.name + ).filter( + met => !context.metrics4tag.includes(met) + ).map( + option => new Object({ label: option, value: option }) + )} + value={ { label: item, value: item } } + /> + } + /> + } + + + +
+ ) +} + + const MetricTagsForm = ({ name=undefined, tag=undefined, @@ -142,7 +264,7 @@ const MetricTagsForm = ({ const addMutation = useMutation(async (values) => await backend.addObject('/api/v2/internal/metrictags/', values)); const deleteMutation = useMutation(async () => await backend.deleteObject(`/api/v2/internal/metrictags/${name}`)) - const { control, getValues, setValue, handleSubmit, formState: { errors } } = useForm({ + const methods = useForm({ defaultValues: { id: `${tag ? tag.id : ""}`, name: `${tag ? tag.name : ""}`, @@ -153,6 +275,8 @@ const MetricTagsForm = ({ resolver: yupResolver(validationSchema) }) + const { control } = methods + const searchItem = useWatch({ control, name: "searchItem" }) const metrics4tag = useWatch({ control, name: "metrics4tag" }) @@ -171,7 +295,7 @@ const MetricTagsForm = ({ } const doChange = () => { - let formValues = getValues() + let formValues = methods.getValues() const sendValues = new Object({ name: formValues.name, @@ -279,178 +403,79 @@ const MetricTagsForm = ({ }} toggle={toggleAreYouSure} > -
- - - - - Name - - - } - /> - - - { message } - - } - /> - - - - - - - - - - - - { - !publicView && - } - - - - - - + + Name - } /> - - - { - metrics4tag.filter( - filteredRow => filteredRow.toLowerCase().includes(searchItem.toLowerCase()) - ).map((item, index) => - - - - - { - !publicView && - - } - - - ) - } - -
#Metric templateActions
- - + + + + +
- { index + 1 } - - { - publicView ? - item - : - - { - let tmpMetrics = getValues("metrics4tag") - tmpMetrics[index] = e.value - setValue("metrics4tag", tmpMetrics) - } } - options={ - allMetrics.map( - met => met.name - ).filter( - met => !metrics4tag.includes(met) - ).map( - option => new Object({ label: option, value: option }) - )} - value={ { label: item, value: item } } - /> - } - /> - } - - - -
- - { - !publicView && -
- { - !addview ? - - : -
- } - -
- } -
+ + + { message } + + } + /> + + + + + + + + + + + { + !publicView && +
+ { + !addview ? + + : +
+ } + +
+ } + + ) } From 6791860d0bec2187d01be6dda3a83087a946a6ac Mon Sep 17 00:00:00 2001 From: Katarina Zailac Date: Thu, 13 Jun 2024 13:10:03 +0200 Subject: [PATCH 04/11] Fix changing metrics when using filtered view --- poem/Poem/frontend/react/MetricTags.js | 3 +- .../react/__tests__/MetricTags.test.js | 82 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/poem/Poem/frontend/react/MetricTags.js b/poem/Poem/frontend/react/MetricTags.js index c69b9f754..b49c48cef 100644 --- a/poem/Poem/frontend/react/MetricTags.js +++ b/poem/Poem/frontend/react/MetricTags.js @@ -184,7 +184,8 @@ const MetricsList = () => { isClearable={ false } onChange={ (e) => { let tmpMetrics = getValues("metrics4tag") - tmpMetrics[index] = e.value + let origIndex = tmpMetrics.findIndex(e => e == item) + tmpMetrics[origIndex] = e.value setValue("metrics4tag", tmpMetrics) } } options={ diff --git a/poem/Poem/frontend/react/__tests__/MetricTags.test.js b/poem/Poem/frontend/react/__tests__/MetricTags.test.js index 10951d4cd..c48de1cf7 100644 --- a/poem/Poem/frontend/react/__tests__/MetricTags.test.js +++ b/poem/Poem/frontend/react/__tests__/MetricTags.test.js @@ -561,6 +561,49 @@ describe("Test metric tags changeview", () => { ) }) + test("Test change metric tags name in filtered view", async () => { + mockChangeObject.mockReturnValueOnce( + Promise.resolve({ ok: true, status: 201, statusText: "CREATED" }) + ) + + renderChangeView() + + await waitFor(() => { + expect(screen.getByRole("button", { name: /save/i })).toBeInTheDocument() + }) + + fireEvent.change(screen.getByPlaceholderText(/search/i), { target: { value: "connect" } }) + + fireEvent.change(screen.getByTestId("name"), { target: { value: "test_tag" } }) + + await waitFor(() => { + expect(screen.getByTestId("form")).toHaveFormValues({name: "test_tag"}) + }) + + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => { + expect(screen.getByRole('dialog', { title: /change/i })).toBeInTheDocument(); + }) + fireEvent.click(screen.getByRole('button', { name: /yes/i })); + + await waitFor(() => { + expect(mockChangeObject).toHaveBeenCalledWith( + "/api/v2/internal/metrictags/", + { id: "4", name: "test_tag", metrics: ["generic.certificate.validity", "generic.http.connect", "generic.tcp.connect"] } + ) + }) + + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictemplate") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictemplate") + expect(NotificationManager.success).toHaveBeenCalledWith( + "Metric tag successfully changed", "Changed", 2000 + ) + }) + test("Test error changing metric tag with error message", async () => { mockChangeObject.mockImplementationOnce(() => { throw Error("400 BAD REQUEST: Metric tag with this name already exists.") @@ -690,6 +733,45 @@ describe("Test metric tags changeview", () => { ) }) + test("Test change metrics for metric tag in filtered view", async () => { + mockChangeObject.mockReturnValueOnce( + Promise.resolve({ ok: true, status: 201, statusText: "CREATED" }) + ) + + renderChangeView() + + await waitFor(() => { + expect(screen.getByRole("button", { name: /save/i })).toBeInTheDocument() + }) + + fireEvent.change(screen.getByPlaceholderText(/search/i), { target: { value: "connect" } }) + + await selectEvent.select(screen.getByText("generic.http.connect"), "argo.AMS-Check") + + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => { + expect(screen.getByRole('dialog', { title: /change/i })).toBeInTheDocument(); + }) + fireEvent.click(screen.getByRole('button', { name: /yes/i })); + + await waitFor(() => { + expect(mockChangeObject).toHaveBeenCalledWith( + "/api/v2/internal/metrictags/", + { id: "4", name: "harmonized", metrics: ["generic.certificate.validity", "argo.AMS-Check", "generic.tcp.connect"] } + ) + }) + + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictemplate") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictemplate") + expect(NotificationManager.success).toHaveBeenCalledWith( + "Metric tag successfully changed", "Changed", 2000 + ) + }) + test("Test display warning messages", async () => { mockChangeObject.mockReturnValueOnce( Promise.resolve({ From 082023f277f00c665bbf2bc18756e6df5a70a192 Mon Sep 17 00:00:00 2001 From: Katarina Zailac Date: Thu, 13 Jun 2024 13:45:57 +0200 Subject: [PATCH 05/11] Fix deleting of metrics in filtered view --- poem/Poem/frontend/react/MetricTags.js | 3 +- .../react/__tests__/MetricTags.test.js | 80 ++++++++++++++++++- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/poem/Poem/frontend/react/MetricTags.js b/poem/Poem/frontend/react/MetricTags.js index b49c48cef..6af7ad63f 100644 --- a/poem/Poem/frontend/react/MetricTags.js +++ b/poem/Poem/frontend/react/MetricTags.js @@ -211,7 +211,8 @@ const MetricsList = () => { data-testid={`remove-${index}`} onClick={() => { let tmpMetrics = context.metrics4tag - tmpMetrics.splice(index, 1) + let origIndex = tmpMetrics.findIndex(e => e == item) + tmpMetrics.splice(origIndex, 1) if (tmpMetrics.length === 0) tmpMetrics = [""] setValue("metrics4tag", tmpMetrics) diff --git a/poem/Poem/frontend/react/__tests__/MetricTags.test.js b/poem/Poem/frontend/react/__tests__/MetricTags.test.js index c48de1cf7..2a8f82e67 100644 --- a/poem/Poem/frontend/react/__tests__/MetricTags.test.js +++ b/poem/Poem/frontend/react/__tests__/MetricTags.test.js @@ -699,8 +699,6 @@ describe("Test metric tags changeview", () => { await selectEvent.select(screen.getByText("generic.certificate.validity"), "argo.AMS-Check") - fireEvent.click(screen.getByTestId("remove-1")) - fireEvent.click(screen.getByTestId("insert-0")) const table = within(screen.getByRole("table")) @@ -718,7 +716,7 @@ describe("Test metric tags changeview", () => { await waitFor(() => { expect(mockChangeObject).toHaveBeenCalledWith( "/api/v2/internal/metrictags/", - { id: "4", name: "harmonized", metrics: ["argo.AMS-Check", "argo.AMSPublisher-Check", "generic.tcp.connect"] } + { id: "4", name: "harmonized", metrics: ["argo.AMS-Check", "argo.AMSPublisher-Check", "generic.http.connect", "generic.tcp.connect"] } ) }) @@ -772,6 +770,82 @@ describe("Test metric tags changeview", () => { ) }) + test("Test delete metrics from metric tag", async () => { + mockChangeObject.mockReturnValueOnce( + Promise.resolve({ ok: true, status: 201, statusText: "CREATED" }) + ) + + renderChangeView() + + await waitFor(() => { + expect(screen.getByRole("button", { name: /save/i })).toBeInTheDocument() + }) + + fireEvent.click(screen.getByTestId("remove-1")) + + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => { + expect(screen.getByRole('dialog', { title: /change/i })).toBeInTheDocument(); + }) + fireEvent.click(screen.getByRole('button', { name: /yes/i })); + + await waitFor(() => { + expect(mockChangeObject).toHaveBeenCalledWith( + "/api/v2/internal/metrictags/", + { id: "4", name: "harmonized", metrics: ["generic.certificate.validity", "generic.tcp.connect"] } + ) + }) + + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictemplate") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictemplate") + expect(NotificationManager.success).toHaveBeenCalledWith( + "Metric tag successfully changed", "Changed", 2000 + ) + }) + + test("Test delete metrics from metric tag if filtered view", async () => { + mockChangeObject.mockReturnValueOnce( + Promise.resolve({ ok: true, status: 201, statusText: "CREATED" }) + ) + + renderChangeView() + + await waitFor(() => { + expect(screen.getByRole("button", { name: /save/i })).toBeInTheDocument() + }) + + fireEvent.change(screen.getByPlaceholderText(/search/i), { target: { value: "connect" } }) + + fireEvent.click(screen.getByTestId("remove-0")) + + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => { + expect(screen.getByRole('dialog', { title: /change/i })).toBeInTheDocument(); + }) + fireEvent.click(screen.getByRole('button', { name: /yes/i })); + + await waitFor(() => { + expect(mockChangeObject).toHaveBeenCalledWith( + "/api/v2/internal/metrictags/", + { id: "4", name: "harmonized", metrics: ["generic.certificate.validity", "generic.tcp.connect"] } + ) + }) + + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictemplate") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictemplate") + expect(NotificationManager.success).toHaveBeenCalledWith( + "Metric tag successfully changed", "Changed", 2000 + ) + }) + test("Test display warning messages", async () => { mockChangeObject.mockReturnValueOnce( Promise.resolve({ From 2bc9b2b73b1cd2cec175eda54e2bdf0c3a1ec796 Mon Sep 17 00:00:00 2001 From: Katarina Zailac Date: Fri, 14 Jun 2024 12:59:07 +0200 Subject: [PATCH 06/11] Change format of metric tags response --- .../api/internal_views/metrictemplates.py | 22 ++++-- poem/Poem/api/tests/test_metrictemplates.py | 72 ++++++++++++++----- 2 files changed, 71 insertions(+), 23 deletions(-) diff --git a/poem/Poem/api/internal_views/metrictemplates.py b/poem/Poem/api/internal_views/metrictemplates.py index 41034fcb7..83c0f19cf 100644 --- a/poem/Poem/api/internal_views/metrictemplates.py +++ b/poem/Poem/api/internal_views/metrictemplates.py @@ -758,7 +758,10 @@ def get(self, request, name=None): return Response({ "id": tag.id, "name": tag.name, - "metrics": sorted([metric.name for metric in metrics]) + "metrics": sorted( + [{"name": metric.name} for metric in metrics], + key=lambda m: m["name"] + ) }) except admin_models.MetricTags.DoesNotExist: @@ -777,7 +780,10 @@ def get(self, request, name=None): data.append({ "id": tag.id, "name": tag.name, - "metrics": sorted([metric.name for metric in metrics]) + "metrics": sorted( + [{"name": metric.name} for metric in metrics], + key=lambda m: m["name"] + ) }) return Response(data) @@ -831,14 +837,16 @@ def post(self, request): if len(missing_metrics) > 0: if len(missing_metrics) == 1: - warn_msg = \ - f"{warn_msg}Metric {list(missing_metrics)[0]} " \ + warn_msg = ( + f"{warn_msg}Metric {list(missing_metrics)[0]} " f"does not exist." + ) else: - warn_msg = "{}Metrics {} do not exist.".format( - warn_msg, - ", ".join(sorted(list(missing_metrics))) + warn_msg = ( + f"{warn_msg}Metrics " + f"{', '.join(sorted(list(missing_metrics)))} " + f"do not exist." ) if warn_msg: diff --git a/poem/Poem/api/tests/test_metrictemplates.py b/poem/Poem/api/tests/test_metrictemplates.py index d6a046773..74ae8e4d4 100644 --- a/poem/Poem/api/tests/test_metrictemplates.py +++ b/poem/Poem/api/tests/test_metrictemplates.py @@ -10022,17 +10022,25 @@ def test_get_metric_tags_admin_superuser(self): { "id": self.tag1.id, "name": "internal", - "metrics": ["argo.AMS-Check"] + "metrics": [{ + "name": "argo.AMS-Check" + }] }, { "id": self.tag3.id, "name": "test_tag1", - "metrics": ["argo.AMS-Check", "argo.EGI-Connectors-Check"] + "metrics": [ + {"name": "argo.AMS-Check"}, + {"name": "argo.EGI-Connectors-Check"} + ] }, { "id": self.tag4.id, "name": "test_tag2", - "metrics": ["argo.AMS-Check", "org.apel.APEL-Pub"] + "metrics": [ + {"name": "argo.AMS-Check"}, + {"name": "org.apel.APEL-Pub"} + ] } ] ) @@ -10052,17 +10060,25 @@ def test_get_metric_tags_admin_regular_user(self): { "id": self.tag1.id, "name": "internal", - "metrics": ["argo.AMS-Check"] + "metrics": [ + {"name": "argo.AMS-Check"} + ] }, { "id": self.tag3.id, "name": "test_tag1", - "metrics": ["argo.AMS-Check", "argo.EGI-Connectors-Check"] + "metrics": [ + {"name": "argo.AMS-Check"}, + {"name": "argo.EGI-Connectors-Check"} + ] }, { "id": self.tag4.id, "name": "test_tag2", - "metrics": ["argo.AMS-Check", "org.apel.APEL-Pub"] + "metrics": [ + {"name": "argo.AMS-Check"}, + {"name": "org.apel.APEL-Pub"} + ] } ] ) @@ -10082,17 +10098,25 @@ def test_get_metric_tags_tenant_superuser(self): { "id": self.tag1.id, "name": "internal", - "metrics": ["argo.AMS-Check"] + "metrics": [ + {"name": "argo.AMS-Check"} + ] }, { "id": self.tag3.id, "name": "test_tag1", - "metrics": ["argo.AMS-Check", "argo.EGI-Connectors-Check"] + "metrics": [ + {"name": "argo.AMS-Check"}, + {"name": "argo.EGI-Connectors-Check"} + ] }, { "id": self.tag4.id, "name": "test_tag2", - "metrics": ["argo.AMS-Check", "org.apel.APEL-Pub"] + "metrics": [ + {"name": "argo.AMS-Check"}, + {"name": "org.apel.APEL-Pub"} + ] } ] ) @@ -10112,17 +10136,25 @@ def test_get_metric_tags_tenant_regular_user(self): { "id": self.tag1.id, "name": "internal", - "metrics": ["argo.AMS-Check"] + "metrics": [ + {"name": "argo.AMS-Check"} + ] }, { "id": self.tag3.id, "name": "test_tag1", - "metrics": ["argo.AMS-Check", "argo.EGI-Connectors-Check"] + "metrics": [ + {"name": "argo.AMS-Check"}, + {"name": "argo.EGI-Connectors-Check"} + ] }, { "id": self.tag4.id, "name": "test_tag2", - "metrics": ["argo.AMS-Check", "org.apel.APEL-Pub"] + "metrics": [ + {"name": "argo.AMS-Check"}, + {"name": "org.apel.APEL-Pub"} + ] } ] ) @@ -10141,7 +10173,9 @@ def test_get_metric_tag_by_name_admin_superuser(self): { "id": self.tag1.id, "name": "internal", - "metrics": ["argo.AMS-Check"] + "metrics": [ + {"name": "argo.AMS-Check"} + ] } ) @@ -10154,7 +10188,9 @@ def test_get_metric_tag_by_name_admin_regular_user(self): { "id": self.tag1.id, "name": "internal", - "metrics": ["argo.AMS-Check"] + "metrics": [ + {"name": "argo.AMS-Check"} + ] } ) @@ -10167,7 +10203,9 @@ def test_get_metric_tag_by_name_tenant_superuser(self): { "id": self.tag1.id, "name": "internal", - "metrics": ["argo.AMS-Check"] + "metrics": [ + {"name": "argo.AMS-Check"} + ] } ) @@ -10180,7 +10218,9 @@ def test_get_metric_tag_by_name_tenant_regular_user(self): { "id": self.tag1.id, "name": "internal", - "metrics": ["argo.AMS-Check"] + "metrics": [ + {"name": "argo.AMS-Check"} + ] } ) From ae9ea40873c95bd50df976294772f37b5cf2f78e Mon Sep 17 00:00:00 2001 From: Katarina Zailac Date: Mon, 17 Jun 2024 14:01:26 +0200 Subject: [PATCH 07/11] Fix all problems with adding and deleting metrics in the UI --- poem/Poem/frontend/react/MetricTags.js | 89 +++++--- .../react/__tests__/MetricTags.test.js | 194 +++++++++++++++--- 2 files changed, 227 insertions(+), 56 deletions(-) diff --git a/poem/Poem/frontend/react/MetricTags.js b/poem/Poem/frontend/react/MetricTags.js index 6af7ad63f..5f85a7ad0 100644 --- a/poem/Poem/frontend/react/MetricTags.js +++ b/poem/Poem/frontend/react/MetricTags.js @@ -1,4 +1,4 @@ -import React, { useContext, useState } from "react" +import React, { useContext, useEffect, useState } from "react" import { useMutation, useQuery, useQueryClient } from "react-query" import { Link, useLocation, useParams, useNavigate } from "react-router-dom" import { fetchMetricTags, fetchMetricTemplates, fetchUserDetails } from "./QueryFunctions" @@ -31,7 +31,7 @@ import { } from "reactstrap" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome" import { faSearch,faPlus, faTimes } from '@fortawesome/free-solid-svg-icons'; -import { Controller, FormProvider, useForm, useFormContext, useWatch } from "react-hook-form" +import { Controller, FormProvider, useFieldArray, useForm, useFormContext, useWatch } from "react-hook-form" import { ErrorMessage } from '@hookform/error-message' import { yupResolver } from '@hookform/resolvers/yup' import * as Yup from "yup" @@ -83,7 +83,7 @@ export const MetricTagsList = (props) => { : row.value.map((metric, i) => - { metric } + { metric.name } ) } @@ -126,7 +126,28 @@ export const MetricTagsList = (props) => { const MetricsList = () => { const context = useContext(MetricTagsContext) - const { control, getValues, setValue } = useFormContext() + const { control, getValues, setValue, resetField } = useFormContext() + + const { fields, insert, remove } = useFieldArray({ control, name: "view_metrics4tag" }) + + const onRemove = (index) => { + let tmp_metrics4tag = [ ...context.metrics4tag ] + let origIndex = tmp_metrics4tag.findIndex(e => e.name == getValues(`view_metrics4tag.${index}.name`)) + tmp_metrics4tag.splice(origIndex, 1) + resetField("metrics4tag") + setValue("metrics4tag", tmp_metrics4tag) + remove(index) + } + + const onInsert = (index) => { + let tmp_metrics4tag = [ ...context.metrics4tag ] + let origIndex = tmp_metrics4tag.findIndex(e => e.name == getValues(`view_metrics4tag.${index}.name`)) + let new_element = { name: "" } + tmp_metrics4tag.splice(origIndex + 1, 0, new_element) + resetField("metrics4tag") + setValue("metrics4tag", tmp_metrics4tag) + insert(index + 1, new_element) + } return ( @@ -160,9 +181,7 @@ const MetricsList = () => { { - context.metrics4tag.filter( - filteredRow => filteredRow.toLowerCase().includes(context.searchItem.toLowerCase()) - ).map((item, index) => + fields.map((item, index) =>
@@ -171,32 +190,31 @@ const MetricsList = () => { { context.publicView ? - item + item.name : { - let tmpMetrics = getValues("metrics4tag") - let origIndex = tmpMetrics.findIndex(e => e == item) - tmpMetrics[origIndex] = e.value - setValue("metrics4tag", tmpMetrics) + let origIndex = getValues("metrics4tag").findIndex(met => met.name == getValues(`view_metrics4tag.${index}.name`) ) + setValue(`metrics4tag.${origIndex}.name`, e.value) + setValue(`view_metrics4tag.${index}.name`, e.value) } } options={ context.allMetrics.map( met => met.name ).filter( - met => !context.metrics4tag.includes(met) + met => !context.metrics4tag.map(met => met.name).includes(met) ).map( option => new Object({ label: option, value: option }) )} - value={ { label: item, value: item } } + value={ { label: field.value, value: field.value } } /> } /> @@ -209,14 +227,7 @@ const MetricsList = () => { size="sm" color="light" data-testid={`remove-${index}`} - onClick={() => { - let tmpMetrics = context.metrics4tag - let origIndex = tmpMetrics.findIndex(e => e == item) - tmpMetrics.splice(origIndex, 1) - if (tmpMetrics.length === 0) - tmpMetrics = [""] - setValue("metrics4tag", tmpMetrics) - }} + onClick={() => onRemove(index)} > @@ -224,11 +235,7 @@ const MetricsList = () => { size="sm" color="light" data-testid={`insert-${index}`} - onClick={() => { - let tmpMetrics = context.metrics4tag - tmpMetrics.splice(index + 1, 0, "") - setValue("metrics4tag", tmpMetrics) - }} + onClick={() => onInsert(index)} > @@ -266,11 +273,14 @@ const MetricTagsForm = ({ const addMutation = useMutation(async (values) => await backend.addObject('/api/v2/internal/metrictags/', values)); const deleteMutation = useMutation(async () => await backend.deleteObject(`/api/v2/internal/metrictags/${name}`)) + const default_metrics4tag = tag?.metrics.length > 0 ? tag.metrics : [{ name: "" }] + const methods = useForm({ defaultValues: { id: `${tag ? tag.id : ""}`, name: `${tag ? tag.name : ""}`, - metrics4tag: tag?.metrics.length > 0 ? tag.metrics : [""], + metrics4tag: default_metrics4tag, + view_metrics4tag: default_metrics4tag, searchItem: "" }, mode: "all", @@ -281,6 +291,23 @@ const MetricTagsForm = ({ const searchItem = useWatch({ control, name: "searchItem" }) const metrics4tag = useWatch({ control, name: "metrics4tag" }) + const view_metrics4tag = useWatch({ control, name: "view_metrics4tag" }) + + useEffect(() => { + if (view_metrics4tag.length === 0) { + methods.setValue("view_metrics4tag", [{ name: "" }]) + } + }, [view_metrics4tag]) + + useEffect(() => { + if (metrics4tag.length === 0) { + methods.setValue("metrics4tag", [{ name: "" }]) + } + }, [metrics4tag]) + + useEffect(() => { + methods.setValue("view_metrics4tag", metrics4tag.filter(e => e.name.toLowerCase().includes(searchItem.toLowerCase()))) + }, [searchItem]) const toggleAreYouSure = () => { setAreYouSureModal(!areYouSureModal) @@ -301,7 +328,7 @@ const MetricTagsForm = ({ const sendValues = new Object({ name: formValues.name, - metrics: formValues.metrics4tag.filter(met => met !== "") + metrics: formValues.metrics4tag.map(met => met.name).filter(met => met !== "") }) if (addview) diff --git a/poem/Poem/frontend/react/__tests__/MetricTags.test.js b/poem/Poem/frontend/react/__tests__/MetricTags.test.js index 2a8f82e67..74d99af23 100644 --- a/poem/Poem/frontend/react/__tests__/MetricTags.test.js +++ b/poem/Poem/frontend/react/__tests__/MetricTags.test.js @@ -45,22 +45,27 @@ const mockListTags = [ { id: "3", name: "internal", - metrics: ["argo.AMSPublisher-Check"] + metrics: [ + { "name": "argo.AMSPublisher-Check" } + ] }, { id: "4", name: "harmonized", metrics: [ - "generic.certificate.validity", - "generic.http.connect", - "generic.tcp.connect", + { "name": "generic.certificate.validity" }, + { "name": "generic.http.connect" }, + { "name": "generic.tcp.connect" } ] }, { id: "2", name: "messaging", - metrics: ["argo.AMS-Check", "argo.AMSPublisher-Check"] - }, + metrics: [ + { "name": "argo.AMS-Check" }, + { "name": "argo.AMSPublisher-Check" } + ] + } ] const mockActiveSession = { @@ -697,15 +702,9 @@ describe("Test metric tags changeview", () => { expect(screen.getByRole("button", { name: /save/i })).toBeInTheDocument() }) - await selectEvent.select(screen.getByText("generic.certificate.validity"), "argo.AMS-Check") - - fireEvent.click(screen.getByTestId("insert-0")) - - const table = within(screen.getByRole("table")) - - const row2 = table.getAllByRole("row")[3] - - await selectEvent.select(within(row2).getByRole("combobox"), "argo.AMSPublisher-Check") + await waitFor(() => { + selectEvent.select(screen.getByText("generic.certificate.validity"), "argo.AMS-Check") + }) fireEvent.click(screen.getByRole('button', { name: /save/i })); await waitFor(() => { @@ -716,7 +715,7 @@ describe("Test metric tags changeview", () => { await waitFor(() => { expect(mockChangeObject).toHaveBeenCalledWith( "/api/v2/internal/metrictags/", - { id: "4", name: "harmonized", metrics: ["argo.AMS-Check", "argo.AMSPublisher-Check", "generic.http.connect", "generic.tcp.connect"] } + { id: "4", name: "harmonized", metrics: ["argo.AMS-Check", "generic.http.connect", "generic.tcp.connect"] } ) }) @@ -846,6 +845,96 @@ describe("Test metric tags changeview", () => { ) }) + test("Test insert new metrics for metric tag", async () => { + mockChangeObject.mockReturnValueOnce( + Promise.resolve({ ok: true, status: 201, statusText: "CREATED" }) + ) + + renderChangeView() + + await waitFor(() => { + expect(screen.getByRole("button", { name: /save/i })).toBeInTheDocument() + }) + + fireEvent.click(screen.getByTestId("insert-0")) + + const table = within(screen.getByRole("table")) + + const row2 = table.getAllByRole("row")[3] + + await selectEvent.select(within(row2).getByRole("combobox"), "argo.AMSPublisher-Check") + + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => { + expect(screen.getByRole('dialog', { title: /change/i })).toBeInTheDocument(); + }) + fireEvent.click(screen.getByRole('button', { name: /yes/i })); + + await waitFor(() => { + expect(mockChangeObject).toHaveBeenCalledWith( + "/api/v2/internal/metrictags/", + { id: "4", name: "harmonized", metrics: ["generic.certificate.validity", "argo.AMSPublisher-Check", "generic.http.connect", "generic.tcp.connect"] } + ) + }) + + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictemplate") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictemplate") + expect(NotificationManager.success).toHaveBeenCalledWith( + "Metric tag successfully changed", "Changed", 2000 + ) + }) + + test("Test insert new metrics for metric tag in filtered view", async () => { + mockChangeObject.mockReturnValueOnce( + Promise.resolve({ ok: true, status: 201, statusText: "CREATED" }) + ) + + renderChangeView() + + await waitFor(() => { + expect(screen.getByRole("button", { name: /save/i })).toBeInTheDocument() + }) + + fireEvent.change(screen.getByPlaceholderText(/search/i), { target: { value: "connect" } }) + + fireEvent.click(screen.getByTestId("insert-0")) + + const table = within(screen.getByRole("table")) + + const row2 = table.getAllByRole("row")[3] + + await waitFor(() => { + selectEvent.select(within(row2).getByRole("combobox"), "argo.AMSPublisher-Check") + }) + + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => { + expect(screen.getByRole('dialog', { title: /change/i })).toBeInTheDocument(); + }) + fireEvent.click(screen.getByRole('button', { name: /yes/i })); + + await waitFor(() => { + expect(mockChangeObject).toHaveBeenCalledWith( + "/api/v2/internal/metrictags/", + { id: "4", name: "harmonized", metrics: ["generic.certificate.validity", "generic.http.connect", "argo.AMSPublisher-Check", "generic.tcp.connect"] } + ) + }) + + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("metrictemplate") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictags") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metric") + expect(queryClient.invalidateQueries).toHaveBeenCalledWith("public_metrictemplate") + expect(NotificationManager.success).toHaveBeenCalledWith( + "Metric tag successfully changed", "Changed", 2000 + ) + }) + test("Test display warning messages", async () => { mockChangeObject.mockReturnValueOnce( Promise.resolve({ @@ -872,7 +961,9 @@ describe("Test metric tags changeview", () => { const row2 = table.getAllByRole("row")[3] - await selectEvent.select(within(row2).getByRole("combobox"), "argo.AMSPublisher-Check") + await waitFor(() => { + selectEvent.select(within(row2).getByRole("combobox"), "argo.AMSPublisher-Check") + }) fireEvent.click(screen.getByRole('button', { name: /save/i })); await waitFor(() => { @@ -933,7 +1024,9 @@ describe("Test metric tags changeview", () => { const row2 = table.getAllByRole("row")[3] - await selectEvent.select(within(row2).getByRole("combobox"), "argo.AMSPublisher-Check") + await waitFor(() => { + selectEvent.select(within(row2).getByRole("combobox"), "argo.AMSPublisher-Check") + }) fireEvent.click(screen.getByRole('button', { name: /save/i })); await waitFor(() => { @@ -1174,6 +1267,11 @@ describe("Test metric tags addview", () => { expect(table.queryByText(/generic/i)).not.toBeInTheDocument() expect(table.queryByText(/argo/i)).not.toBeInTheDocument() + expect(table.queryByText("generic.certificate.validity")).not.toBeInTheDocument() + expect(table.queryByText("generic.http.connect")).not.toBeInTheDocument() + expect(table.queryByText("generic.tcp.connect")).not.toBeInTheDocument() + expect(table.queryByText("argo.AMS-Check")).not.toBeInTheDocument() + expect(table.queryByText("argo.AMSPublisher-Check")).not.toBeInTheDocument() selectEvent.openMenu(input) expect(table.getByText("generic.certificate.validity")).toBeInTheDocument() expect(table.getByText("generic.http.connect")).toBeInTheDocument() @@ -1250,24 +1348,46 @@ describe("Test metric tags addview", () => { const row1 = table.getAllByRole("row")[2] const input1 = within(row1).getByRole("combobox") - await selectEvent.select(input1, "generic.certificate.validity") + await waitFor(() => { + selectEvent.select(input1, "generic.certificate.validity") + }) + + await waitFor(() => { + expect(table.queryByText("generic.http.connect")).not.toBeInTheDocument() + }) fireEvent.click(table.getByTestId("insert-0")) const row2 = table.getAllByRole("row")[3] const input2 = within(row2).getByRole("combobox") - await selectEvent.select(input2, "generic.http.connect") + await waitFor(() => { + selectEvent.select(input2, "generic.http.connect") + }) + + await waitFor(() => { + expect(table.queryByText("generic.tcp.connect")).not.toBeInTheDocument() + }) fireEvent.click(table.getByTestId("insert-1")) const row3 = table.getAllByRole("row")[4] const input3 = within(row3).getByRole("combobox") - await selectEvent.select(input3, "generic.tcp.connect") + await waitFor(() => { + selectEvent.select(input3, "generic.tcp.connect") + }) + + await waitFor(() => { + expect(table.getByText("generic.tcp.connect")).toBeInTheDocument() + }) fireEvent.click(table.getByTestId("remove-1")) + await waitFor(() => { + expect(table.queryByText("generic.http.connect")).not.toBeInTheDocument() + }) + await selectEvent.select(table.getByText("generic.tcp.connect"), "argo.AMS-Check") fireEvent.click(screen.getByRole('button', { name: /save/i })); @@ -1317,13 +1437,25 @@ describe("Test metric tags addview", () => { const row1 = table.getAllByRole("row")[2] const input1 = within(row1).getByRole("combobox") - await selectEvent.select(input1, "generic.tcp.connect") + await waitFor(() => { + selectEvent.select(input1, "generic.tcp.connect") + }) + + await waitFor(() => { + expect(table.getByText("generic.tcp.connect")).toBeInTheDocument() + }) fireEvent.click(table.getByTestId("insert-0")) const row2 = table.getAllByRole("row")[3] - await selectEvent.select(within(row2).getByRole("combobox"), "generic.http.connect") + await waitFor(() => { + selectEvent.select(within(row2).getByRole("combobox"), "generic.http.connect") + }) + + await waitFor(() => { + expect(table.getByText("generic.http.connect")).toBeInTheDocument() + }) fireEvent.click(screen.getByRole('button', { name: /save/i })); await waitFor(() => { @@ -1381,13 +1513,25 @@ describe("Test metric tags addview", () => { const row1 = table.getAllByRole("row")[2] const input1 = within(row1).getByRole("combobox") - await selectEvent.select(input1, "generic.tcp.connect") + await waitFor(() => { + selectEvent.select(input1, "generic.tcp.connect") + }) + + await waitFor(() => { + expect(table.getByText("generic.tcp.connect")).toBeInTheDocument() + }) fireEvent.click(table.getByTestId("insert-0")) const row2 = table.getAllByRole("row")[3] - await selectEvent.select(within(row2).getByRole("combobox"), "generic.http.connect") + await waitFor(() => { + selectEvent.select(within(row2).getByRole("combobox"), "generic.http.connect") + }) + + await waitFor(() => { + expect(table.getByText("generic.http.connect")).toBeInTheDocument() + }) fireEvent.click(screen.getByRole('button', { name: /save/i })); await waitFor(() => { From 730c18834353854b32f734641ac5c187f2113a79 Mon Sep 17 00:00:00 2001 From: Katarina Zailac Date: Mon, 17 Jun 2024 14:17:26 +0200 Subject: [PATCH 08/11] When importing internal metrics, leave out the ones that have reached their EOL --- .../poem/management/commands/import_internal_metrics.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/poem/Poem/poem/management/commands/import_internal_metrics.py b/poem/Poem/poem/management/commands/import_internal_metrics.py index cea27f5e0..6b4b3ef4b 100644 --- a/poem/Poem/poem/management/commands/import_internal_metrics.py +++ b/poem/Poem/poem/management/commands/import_internal_metrics.py @@ -15,7 +15,8 @@ def handle(self, *args, **kwargs): tenant = connection.tenant internal_metrics = [ mt.name for mt in admin_models.MetricTemplate.objects.all() - if 'internal' in [tag.name for tag in mt.tags.all()] + if 'internal' in [tag.name for tag in mt.tags.all()] and + "eol" not in [tag.name for tag in mt.tags.all()] ] if len(internal_metrics) > 0: try: @@ -32,8 +33,8 @@ def handle(self, *args, **kwargs): get_user_model().DoesNotExist, NoSectionError, NoOptionError ): self.stderr.write( - 'Super user for tenant {} is not defined.\n' - 'Internal metrics not imported.'.format(tenant.name) + f'Super user for tenant {tenant.name} is not defined.\n' + f'Internal metrics not imported.' ) else: From 0923af9e60f4e6ccb7eac26e6215c989877c7ff4 Mon Sep 17 00:00:00 2001 From: Katarina Zailac Date: Mon, 17 Jun 2024 15:41:27 +0200 Subject: [PATCH 09/11] Treat internal metrics same as all the others when syncing metrics --- poem/Poem/api/tests/test_helpers.py | 36 ++++++++++++++++++---------- poem/Poem/helpers/metrics_helpers.py | 22 +++++++---------- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/poem/Poem/api/tests/test_helpers.py b/poem/Poem/api/tests/test_helpers.py index 87b89332f..9ba84e53b 100644 --- a/poem/Poem/api/tests/test_helpers.py +++ b/poem/Poem/api/tests/test_helpers.py @@ -3587,10 +3587,13 @@ def test_sync_active_metrics(self, mock_get_metrics): self.assertEqual(err, []) self.assertEqual(unavailable, []) self.assertEqual( - sorted(deleted), - ["eu.egi.cloud.OpenStack-Swift", "test.Metric-Template"] + sorted(deleted), [ + "argo.poem-tools.check", + "eu.egi.cloud.OpenStack-Swift", + "test.Metric-Template" + ] ) - self.assertEqual(len(poem_models.Metric.objects.all()), 5) + self.assertEqual(len(poem_models.Metric.objects.all()), 4) self.assertRaises( poem_models.Metric.DoesNotExist, poem_models.Metric.objects.get, @@ -3635,10 +3638,13 @@ def test_sync_passive_metrics(self, mock_get_metrics): self.assertEqual(err, []) self.assertEqual(unavailable, []) self.assertEqual( - sorted(deleted), - ["eu.egi.cloud.OpenStack-Swift", "test.Metric-Template"] + sorted(deleted), [ + "argo.poem-tools.check", + "eu.egi.cloud.OpenStack-Swift", + "test.Metric-Template" + ] ) - self.assertEqual(len(poem_models.Metric.objects.all()), 5) + self.assertEqual(len(poem_models.Metric.objects.all()), 4) self.assertRaises( poem_models.Metric.DoesNotExist, poem_models.Metric.objects.get, @@ -3683,10 +3689,13 @@ def test_sync_metrics_with_warning(self, mock_get_metrics): self.assertEqual(err, []) self.assertEqual(unavailable, []) self.assertEqual( - sorted(deleted), - ["eu.egi.cloud.OpenStack-Swift", "test.Metric-Template"] + sorted(deleted), [ + "argo.poem-tools.check", + "eu.egi.cloud.OpenStack-Swift", + "test.Metric-Template" + ] ) - self.assertEqual(len(poem_models.Metric.objects.all()), 5) + self.assertEqual(len(poem_models.Metric.objects.all()), 4) self.assertRaises( poem_models.Metric.DoesNotExist, poem_models.Metric.objects.get, @@ -3731,10 +3740,13 @@ def test_sync_metrics_if_version_unavailable(self, mock_get_metrics): self.assertEqual(err, []) self.assertEqual(unavailable, ["test.MetricTemplate"]) self.assertEqual( - sorted(deleted), - ["eu.egi.cloud.OpenStack-Swift", "test.Metric-Template"] + sorted(deleted), [ + "argo.poem-tools.check", + "eu.egi.cloud.OpenStack-Swift", + "test.Metric-Template" + ] ) - self.assertEqual(len(poem_models.Metric.objects.all()), 4) + self.assertEqual(len(poem_models.Metric.objects.all()), 3) self.assertRaises( poem_models.Metric.DoesNotExist, poem_models.Metric.objects.get, diff --git a/poem/Poem/helpers/metrics_helpers.py b/poem/Poem/helpers/metrics_helpers.py index c350fe4a3..d8bb25a30 100644 --- a/poem/Poem/helpers/metrics_helpers.py +++ b/poem/Poem/helpers/metrics_helpers.py @@ -386,9 +386,6 @@ def sync_metrics(tenant, user): key for key in get_metrics_in_profiles(tenant=tenant) ] metrics = poem_models.Metric.objects.all().values_list("name", flat=True) - internal = admin_models.MetricTemplate.objects.filter( - tags__name="internal" - ).values_list("name", flat=True) missing_metrics = list(set(metrics_in_profiles).difference(set(metrics))) extra_metrics = list(set(metrics).difference(set(metrics_in_profiles))) @@ -399,15 +396,14 @@ def sync_metrics(tenant, user): deleted = list() for metric in extra_metrics: - if metric not in internal: - m = poem_models.Metric.objects.get(name=metric) - poem_models.TenantHistory.objects.filter( - object_id=m.id, - content_type=ContentType.objects.get_for_model( - poem_models.Metric - ) - ).delete() - m.delete() - deleted.append(metric) + m = poem_models.Metric.objects.get(name=metric) + poem_models.TenantHistory.objects.filter( + object_id=m.id, + content_type=ContentType.objects.get_for_model( + poem_models.Metric + ) + ).delete() + m.delete() + deleted.append(metric) return imported, warn, err, unavailable, deleted From fefe7883e00d3dce5c62ea633e9055a03a1d6aab Mon Sep 17 00:00:00 2001 From: Katarina Zailac Date: Tue, 18 Jun 2024 08:45:53 +0200 Subject: [PATCH 10/11] Fix handling of state in list of metric templates --- poem/Poem/frontend/react/Metrics.js | 1 + 1 file changed, 1 insertion(+) diff --git a/poem/Poem/frontend/react/Metrics.js b/poem/Poem/frontend/react/Metrics.js index 7be3234f1..9132ed9a1 100644 --- a/poem/Poem/frontend/react/Metrics.js +++ b/poem/Poem/frontend/react/Metrics.js @@ -398,6 +398,7 @@ export const ListOfMetrics = (props) => { NotifyWarn({ msg: data.warning, title: 'Deleted' }) setSelectAll(0); + setSelected({}); queryClient.invalidateQueries('metrictemplate'); queryClient.invalidateQueries('public_metrictemplate'); }, From 3a0b9c3cbcdec4967523b32738a4ace934e0648c Mon Sep 17 00:00:00 2001 From: Katarina Zailac Date: Tue, 18 Jun 2024 10:49:25 +0200 Subject: [PATCH 11/11] Add import .csv button in metric profile addview --- poem/Poem/frontend/react/MetricProfiles.js | 2 +- .../react/__tests__/MetricProfiles.test.js | 248 +++++++++++++++++- 2 files changed, 247 insertions(+), 3 deletions(-) diff --git a/poem/Poem/frontend/react/MetricProfiles.js b/poem/Poem/frontend/react/MetricProfiles.js index 1aac5429c..65168151a 100644 --- a/poem/Poem/frontend/react/MetricProfiles.js +++ b/poem/Poem/frontend/react/MetricProfiles.js @@ -481,7 +481,7 @@ const MetricProfilesForm = ({ infoview={historyview} submitperm={write_perm} extra_button={ - !addview && + !publicView && setDropdownOpen(!dropdownOpen)}> CSV diff --git a/poem/Poem/frontend/react/__tests__/MetricProfiles.test.js b/poem/Poem/frontend/react/__tests__/MetricProfiles.test.js index a34e8c080..3c07cf974 100644 --- a/poem/Poem/frontend/react/__tests__/MetricProfiles.test.js +++ b/poem/Poem/frontend/react/__tests__/MetricProfiles.test.js @@ -1466,6 +1466,7 @@ describe('Tests for metric profiles changeview', () => { expect(metricInstances.getAllByTestId(/insert-/i)).toHaveLength(2); expect(screen.queryByText('Must be one of predefined metrics')).not.toBeInTheDocument() + }) test('Test import csv with nonexisting metrics', async () => { @@ -3549,7 +3550,7 @@ describe('Tests for metric profile addview', () => { expect(screen.queryByRole('button', { name: /delete/i })).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: /clone/i })).not.toBeInTheDocument(); - expect(screen.queryByRole('button', { name: /csv/i })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /csv/i })).toBeInTheDocument(); }) test('Test that page renders properly for combined tenant', async () => { @@ -3598,7 +3599,7 @@ describe('Tests for metric profile addview', () => { expect(screen.queryByRole('button', { name: /delete/i })).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: /clone/i })).not.toBeInTheDocument(); - expect(screen.queryByRole('button', { name: /csv/i })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /csv/i })).toBeInTheDocument(); }) test("Test add main profile info", async () => { @@ -3745,6 +3746,249 @@ describe('Tests for metric profile addview', () => { expect(row1.getAllByText("Select...")).toHaveLength(2) }) + test("Test import csv successfully", async () => { + mockAddMetricProfile.mockReturnValueOnce( + Promise.resolve({ + status: { + message: 'Metric profile Created', + code: '200' + }, + data: { + id: 'va0ahsh6-6rs0-14ho-xlh9-wahso4hie7iv', + links: { + self: 'string' + } + } + }) + ) + + renderAddView() + + await waitFor(() => { + expect(screen.getByRole("button", { name: /save/i })).toBeInTheDocument() + }) + + fireEvent.change(screen.getByTestId('name'), { target: { value: 'NEW_PROFILE' } }); + fireEvent.change(screen.getByLabelText(/description/i), { target: { value: 'New central ARGO_MON profile.' } }); + + await waitFor(() => { + selectEvent.select(screen.getAllByText("Select...")[0], 'ARGO') + }) + + fireEvent.click(screen.getByRole('button', { name: /csv/i })); + fireEvent.click(screen.getByRole('menuitem', { name: /import/i })); + + const csv = 'service,metric\r\norg.opensciencegrid.htcondorce,ch.cern.HTCondorCE-JobState\r\norg.opensciencegrid.htcondorce,ch.cern.HTCondorCE-JobSubmit\r\n'; + + const content = new Blob([csv], { type: 'text/csv;charset=UTF-8' }); + const file = new File([content], 'profile.csv', { type: 'text/csv;charset=UTF-8' }); + const input = screen.getByTestId('file_input'); + + await waitFor(() => { + useEvent.upload(input, file); + }) + + await waitFor(() => { + expect(input.files[0]).toStrictEqual(file) + }) + expect(input.files.item(0)).toStrictEqual(file) + expect(input.files).toHaveLength(1) + + await waitFor(() => { + fireEvent.load(screen.getByTestId('file_input')) + }) + + const metricInstances = within(screen.getByRole('table')); + const rows = metricInstances.getAllByRole('row'); + expect(rows).toHaveLength(4); + const row1 = within(rows[2]) + const row2 = within(rows[3]) + + expect(row1.getByText("org.opensciencegrid.htcondorce")).toBeInTheDocument() + expect(row1.getByText("ch.cern.HTCondorCE-JobState")).toBeInTheDocument() + expect(row2.getByText("org.opensciencegrid.htcondorce")).toBeInTheDocument() + expect(row2.getByText("ch.cern.HTCondorCE-JobSubmit")).toBeInTheDocument() + + expect(metricInstances.getAllByTestId(/remove-/i)).toHaveLength(2); + expect(metricInstances.getAllByTestId(/insert-/i)).toHaveLength(2); + + expect(screen.queryByText('Must be one of predefined metrics')).not.toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: /save/i })) + await waitFor(() => { + expect(screen.getByRole('dialog', { title: /add/i })).toBeInTheDocument(); + }) + fireEvent.click(screen.getByRole('button', { name: /yes/i })); + + await waitFor(() => { + expect(mockAddMetricProfile).toHaveBeenCalledWith({ + description: 'New central ARGO_MON profile.', + name: 'NEW_PROFILE', + services: [ + { + service: "org.opensciencegrid.htcondorce", + metrics: [ + "ch.cern.HTCondorCE-JobState", + "ch.cern.HTCondorCE-JobSubmit" + ] + } + ] + }) + }) + }) + + test("Test import csv with nonexisting metrics", async () => { + renderAddView() + + await waitFor(() => { + expect(screen.getByRole("button", { name: /save/i })).toBeInTheDocument() + }) + + fireEvent.click(screen.getByRole('button', { name: /csv/i })); + fireEvent.click(screen.getByRole('menuitem', { name: /import/i })); + + const csv = 'service,metric\r\norg.opensciencegrid.htcondorce,ch.cern.HTCondorCE-CertValidity\r\norg.opensciencegrid.htcondorce,ch.cern.HTCondorCE-JobSubmit\r\n'; + + const content = new Blob([csv], { type: 'text/csv;charset=UTF-8' }); + const file = new File([content], 'profile.csv', { type: 'text/csv;charset=UTF-8' }); + const input = screen.getByTestId('file_input'); + + await waitFor(() => { + useEvent.upload(input, file); + }) + + await waitFor(() => { + expect(input.files[0]).toStrictEqual(file) + }) + expect(input.files.item(0)).toStrictEqual(file) + expect(input.files).toHaveLength(1) + + await waitFor(() => { + fireEvent.load(screen.getByTestId('file_input')) + }) + + const metricInstances = within(screen.getByRole('table')); + const rows = metricInstances.getAllByRole('row'); + expect(rows).toHaveLength(4); + const row1 = within(rows[2]) + const row2 = within(rows[3]) + + expect(row1.getByText("org.opensciencegrid.htcondorce")).toBeInTheDocument() + expect(row1.getByText("ch.cern.HTCondorCE-CertValidity")).toBeInTheDocument() + expect(row2.getByText("org.opensciencegrid.htcondorce")).toBeInTheDocument() + expect(row2.getByText("ch.cern.HTCondorCE-JobSubmit")).toBeInTheDocument() + + expect(metricInstances.getAllByTestId(/remove-/i)).toHaveLength(2); + expect(metricInstances.getAllByTestId(/insert-/i)).toHaveLength(2); + + expect(screen.queryByText('Must be one of predefined metrics')).toBeInTheDocument() + }) + + test("Test import csv, make some changes and save profile successfully", async () => { + mockAddMetricProfile.mockReturnValueOnce( + Promise.resolve({ + status: { + message: 'Metric profile Created', + code: '200' + }, + data: { + id: 'va0ahsh6-6rs0-14ho-xlh9-wahso4hie7iv', + links: { + self: 'string' + } + } + }) + ) + + renderAddView() + + await waitFor(() => { + expect(screen.getByRole("button", { name: /save/i })).toBeInTheDocument() + }) + + fireEvent.change(screen.getByTestId('name'), { target: { value: 'NEW_PROFILE' } }); + fireEvent.change(screen.getByLabelText(/description/i), { target: { value: 'New central ARGO_MON profile.' } }); + + await waitFor(() => { + selectEvent.select(screen.getAllByText("Select...")[0], 'ARGO') + }) + + fireEvent.click(screen.getByRole('button', { name: /csv/i })); + fireEvent.click(screen.getByRole('menuitem', { name: /import/i })); + + const csv = 'service,metric\r\norg.opensciencegrid.htcondorce,ch.cern.HTCondorCE-JobState\r\norg.opensciencegrid.htcondorce,ch.cern.HTCondorCE-JobSubmit\r\n'; + + const content = new Blob([csv], { type: 'text/csv;charset=UTF-8' }); + const file = new File([content], 'profile.csv', { type: 'text/csv;charset=UTF-8' }); + const input = screen.getByTestId('file_input'); + + await waitFor(() => { + useEvent.upload(input, file); + }) + + await waitFor(() => { + expect(input.files[0]).toStrictEqual(file) + }) + expect(input.files.item(0)).toStrictEqual(file) + expect(input.files).toHaveLength(1) + + await waitFor(() => { + fireEvent.load(screen.getByTestId('file_input')) + }) + + await waitFor(() => { + expect(within(screen.getByRole("table")).getAllByRole("row")).toHaveLength(4) + }) + + fireEvent.click(screen.getByTestId("insert-0")) + + const metricInstances = within(screen.getByRole('table')); + var rows = metricInstances.getAllByRole('row'); + const row2 = within(rows[3]) + + await waitFor(() => { + selectEvent.select(row2.getAllByText("Select...")[0], "eu.argo.ams") + }) + + await waitFor(() => { + selectEvent.select(row2.getAllByText("Select...")[1], "argo.AMS-Check") + }) + + await waitFor(() => { + expect(screen.queryByText(/duplicated/i)).not.toBeInTheDocument() + }) + + expect(screen.queryByText('Must be one of predefined metrics')).not.toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: /save/i })) + await waitFor(() => { + expect(screen.getByRole('dialog', { title: /add/i })).toBeInTheDocument(); + }) + fireEvent.click(screen.getByRole('button', { name: /yes/i })); + + await waitFor(() => { + expect(mockAddMetricProfile).toHaveBeenCalledWith({ + description: 'New central ARGO_MON profile.', + name: 'NEW_PROFILE', + services: [ + { + service: "eu.argo.ams", + metrics: [ + "argo.AMS-Check" + ] + }, + { + service: "org.opensciencegrid.htcondorce", + metrics: [ + "ch.cern.HTCondorCE-JobState", + "ch.cern.HTCondorCE-JobSubmit" + ] + } + ] + }) + }) + }) + test("Test importing tuples from constituting tenants for combined tenant", async () => { renderAddView(true)