Migrated last Components and Configuration area #1708

johnsimons · 2024-02-09T04:48:25Z

This PR pretty much migrates all remaining areas with the exception of "Failed Messages" area.

~~As part of this PR, I also removed the calls to initialise certain services from App.vue, which means we rely on modules to execute them.~~
Reverted ☝️

johnsimons · 2024-02-09T06:56:59Z

src/ServicePulse.Host/vue/src/components/ConfirmDialog.vue


 function confirm() {
-  emit("confirm", settings);
+  emit("confirm");


the settings were not used, so I removed them

johnsimons · 2024-02-09T06:57:45Z

src/ServicePulse.Host/vue/src/components/PageHeader.vue


 const baseUrl = window.defaultConfig.base_url;

-function subIsActive(input, exact) {
-  const paths = Array.isArray(input) ? input : [input];


nothing was passing an array, so removed that code

johnsimons · 2024-02-09T06:58:19Z

src/ServicePulse.Host/vue/src/components/configuration/EndpointConnection.vue

-// The event is only actually emitted on the RetryRedirects.vue component
-// but if we don't include it, the console will show warnings about not being able to
-// subscribe to this event
-defineEmits(["redirectCountUpdated"]);


using a reactive exported const instead

johnsimons · 2024-02-09T06:59:54Z

...rvicePulse.Host/vue/src/components/configuration/HealthCheckNotifications_ConfigureEmail.vue

@@ -20,36 +18,36 @@ const enable_tls = ref(settings.enable_tls);
 const from = ref(settings.from);
 const to = ref(settings.to);

-const emailRe = /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
+const emailRe = /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;


unnecessary escape

johnsimons · 2024-02-09T07:07:36Z

src/ServicePulse.Host/vue/src/components/configuration/RetryRedirects.vue


 const isExpired = licenseStatus.isExpired;

-const emit = defineEmits(["redirectCountUpdated"]);


this now in redirectCountUpdated

src/ServicePulse.Host/vue/src/App.vue

src/ServicePulse.Host/vue/src/components/configuration/PlatformConnections.vue

src/ServicePulse.Host/vue/src/resources/UpdateEmailNotificationsSettingsRequest.ts

src/ServicePulse.Host/vue/src/App.vue

src/ServicePulse.Host/vue/src/components/configuration/EndpointConnection.vue

… the other initialisation

into john/cheese

PhilBastian · 2024-03-01T00:46:37Z

src/ServicePulse.Host/vue/src/composables/serviceServiceControl.ts

@@ -252,11 +247,13 @@ async function getServiceControlConnection() {

 async function getMonitoringConnection() {
  try {
-    const [, data] = await useTypedFetchFromMonitoring<{ Metrics: MetricsConnectionDetails }>("connection");
-    return data;
+    if (!useIsMonitoringDisabled()) {


not required, is in useTypedFetchFromMonitoring, which will return an undefined response and data

PhilBastian · 2024-03-01T00:48:59Z

src/ServicePulse.Host/vue/src/composables/serviceServiceControl.ts

  } catch {
    connections.monitoring.errors = [`Error SC Monitoring instance at ${monitoringUrl.value}connection`];
-    return null;
+    return undefined;


does not returning anything also return undefined implicitly? That's what's happening currently if the if clause above returns false

also in getServiceControlConnection this same pattern is returning null rather than undefined, should be consistent

fixed, anything else @PhilBastian ?

PhilBastian · 2024-03-01T00:49:22Z

src/ServicePulse.Host/vue/src/composables/serviceServiceControl.ts

-  } catch {
-    return null;
-  }
+  } catch {}


remove try/catch

should we rename the function? It's not getting the monitoring version, it's setting it in the environment settings

fixed and renamed

PhilBastian · 2024-03-01T00:57:22Z

src/ServicePulse.Host/vue/src/components/configuration/RetryRedirectEdit.vue

@@ -63,6 +77,8 @@ function edit() {
 function close() {
  emit("cancel");
 }
+
+function save() {}


need to figure that one out!

This seems to be broken in master

ServicePulse/src/ServicePulse.Host/vue/src/components/configuration/RetryRedirectEdit.vue

Line 77 in fee0e35

<form name="redirectForm" novalidate @submit.prevent="save">

I actually think this fix is ok, it seems the logic is in the onclick of the buttons.
Thoughts @PhilBastian ?

shouldn't the call save on submit logic be removed? I would guess whomever migrated this originally intended to have it have form submit functionality (which isn't in angular), and then forgot

PhilBastian · 2024-03-01T01:07:40Z

src/ServicePulse.Host/vue/src/composables/serviceServiceControl.ts

+}
+
+interface MetricsConnectionDetails {
+  Enabled: boolean;


should be camelCase

The payload from the server is not camelCase, see https://particularsoftware.slack.com/archives/C368VHCTT/p1706660541812479

could use https://www.npmjs.com/package/camelcase-keys on the API response (on all API responses to be consistent)

possibly, or we fix the server side 😄

PhilBastian · 2024-03-01T01:10:40Z

src/ServicePulse.Host/vue/src/composables/serviceServiceControl.ts

+  //NOTE to eliminate success msg showing everytime the screen is refreshed
+  if (newValue !== oldValue) {
+    if (newValue) {
+      useShowToast(TYPE.ERROR, "Error", "Could not connect to ServiceControl at " + serviceControlUrl.value + '. <a class="btn btn-default" href="/#/configuration/connections">View connection settings</a>');


convert to interpolation

PhilBastian · 2024-03-01T01:11:12Z

src/ServicePulse.Host/vue/src/composables/serviceServiceControl.ts

+  //NOTE to eliminate success msg showing everytime the screen is refreshed
+  if (newValue !== oldValue) {
+    if (newValue) {
+      useShowToast(TYPE.ERROR, "Error", "Could not connect to the ServiceControl Monitoring service at " + monitoringUrl.value + '. <a class="btn btn-default" href="/#/configuration/connections">View connection settings</a>');


convert to interpolation

johnsimons self-assigned this Feb 9, 2024

johnsimons added 3 commits February 9, 2024 16:49

Migrated Components and Configuration

b0d1642

A few small fixes

7e6239c

Fixed a few issues

de5627b

johnsimons force-pushed the john/cheese branch from 1f47bb2 to de5627b Compare February 9, 2024 06:49

johnsimons commented Feb 9, 2024

View reviewed changes

johnsimons marked this pull request as ready for review February 9, 2024 08:18

johnsimons requested a review from PhilBastian February 9, 2024 08:18

mikeminutillo reviewed Feb 9, 2024

View reviewed changes

Feedback from code review

46f849d

PhilBastian approved these changes Feb 12, 2024

View reviewed changes

src/ServicePulse.Host/vue/src/App.vue Show resolved Hide resolved

src/ServicePulse.Host/vue/src/components/configuration/EndpointConnection.vue Show resolved Hide resolved

Merge branch 'master' into john/cheese

74d4be4

soujay self-requested a review February 21, 2024 22:54

cquirosj changed the base branch from master to merge_monitoring_master February 27, 2024 20:50

cquirosj changed the base branch from merge_monitoring_master to master February 27, 2024 21:52

cquirosj force-pushed the john/cheese branch from ecb1d5e to 74d4be4 Compare February 27, 2024 22:09

cquirosj and others added 7 commits February 27, 2024 16:22

Merge branch 'master' into john/cheese

7063712

We need to ensure useServiceControlUrls() is the first one before all…

ca2103e

… the other initialisation

fix events link in angular header

c04f8a9

Merge branch 'john/cheese' of https://github.com/Particular/ServicePulse

412ca23

into john/cheese

Merge branch 'master' into john/cheese

651738e

Merge branch 'john/cheese'

35a787e

Merge branch 'master' into john/cheese

51af371

PhilBastian approved these changes Mar 1, 2024

View reviewed changes

johnsimons added 2 commits March 1, 2024 11:41

Some fixes based on feedback

cad1bed

Fixed toast

83ff534

Merge branch 'master' into john/cheese

db2deb8

johnsimons merged commit d7c4ae0 into master Mar 5, 2024
5 checks passed

johnsimons deleted the john/cheese branch March 5, 2024 23:41

PhilBastian mentioned this pull request Mar 6, 2024

Typescript merge fixes #1770

Merged

cquirosj added this to the 1.38.2 milestone Mar 20, 2024

TravisNickels added Type: Refactoring Type: Refactoring Type: Improvement Type: Improvement labels Mar 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrated last Components and Configuration area #1708

Migrated last Components and Configuration area #1708

johnsimons commented Feb 9, 2024 •

edited

Loading

johnsimons Feb 9, 2024

johnsimons Feb 9, 2024

johnsimons Feb 9, 2024

johnsimons Feb 9, 2024

johnsimons Feb 9, 2024

PhilBastian Mar 1, 2024

johnsimons Mar 1, 2024

PhilBastian Mar 1, 2024

PhilBastian Mar 1, 2024

johnsimons Mar 1, 2024

PhilBastian Mar 1, 2024

PhilBastian Mar 1, 2024

johnsimons Mar 1, 2024

PhilBastian Mar 1, 2024

johnsimons Mar 1, 2024

johnsimons Mar 1, 2024

johnsimons Mar 1, 2024

PhilBastian Mar 5, 2024

PhilBastian Mar 1, 2024

johnsimons Mar 1, 2024

PhilBastian Mar 1, 2024

johnsimons Mar 1, 2024

PhilBastian Mar 1, 2024

johnsimons Mar 1, 2024

PhilBastian Mar 1, 2024

johnsimons Mar 1, 2024


		const isExpired = licenseStatus.isExpired;

		const emit = defineEmits(["redirectCountUpdated"]);

Migrated last Components and Configuration area #1708

Migrated last Components and Configuration area #1708

Conversation

johnsimons commented Feb 9, 2024 • edited Loading

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

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

johnsimons commented Feb 9, 2024 •

edited

Loading