Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

this is 2024. * observers no more. #7715

Merged
merged 19 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 109 additions & 2 deletions e2e/tests/functional/plugins/conditionSet/conditionSet.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ test.describe('Basic Condition Set Use', () => {

// Validate that the condition set is evaluating and outputting
// the correct value when the underlying telemetry subscription is active.
let outputValue = page.locator('[aria-label="Current Output Value"]');
let outputValue = page.locator('[aria-label="Condition Set Current Output Value"]');
davetsay marked this conversation as resolved.
Show resolved Hide resolved
await expect(outputValue).toHaveText('false');

await page.goto(exampleTelemetry.url);
Expand Down Expand Up @@ -462,7 +462,7 @@ test.describe('Basic Condition Set Use', () => {

// Validate that the condition set is evaluating and outputting
// the correct value when the underlying telemetry subscription is active.
let outputValue = page.locator('[aria-label="Current Output Value"]');
let outputValue = page.locator('[aria-label="Condition Set Current Output Value"]');
davetsay marked this conversation as resolved.
Show resolved Hide resolved
await expect(outputValue).toHaveText('false');

await page.goto(exampleTelemetry.url);
Expand All @@ -475,3 +475,110 @@ test.describe('Basic Condition Set Use', () => {
});
});
});

test.describe('Condition Set Composition', () => {
let conditionSet;
let exampleTelemetry;

test.beforeEach(async ({ page }) => {
await page.goto('./', { waitUntil: 'domcontentloaded' });

// Create Condition Set
conditionSet = await createDomainObjectWithDefaults(page, {
type: 'Condition Set',
name: 'Condition Set with Telemetry and Conditions'
davetsay marked this conversation as resolved.
Show resolved Hide resolved
});

// Create Telemetry Object
exampleTelemetry = await createExampleTelemetryObject(page);
davetsay marked this conversation as resolved.
Show resolved Hide resolved

// Make Link from Telemetry Object to Overlay Plot
await page.locator('button[title="More actions"]').click();
davetsay marked this conversation as resolved.
Show resolved Hide resolved

// Select 'Create Link' from dropdown
await page.getByRole('menuitem', { name: 'Create Link' }).click();

// Search and Select for Condition Set within Create Modal
await page.getByRole('dialog').getByRole('searchbox', { name: 'Search Input' }).click();
await page
.getByRole('dialog')
.getByRole('searchbox', { name: 'Search Input' })
.fill(conditionSet.name);
await page
.getByRole('treeitem', { name: new RegExp(conditionSet.name) })
.locator('a')
.click();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's an easier way to do this now? Check with Playwright's tooling. The generated locator should work and be simpler

await page.getByRole('button', { name: 'Save' }).click();

// Edit Condition Set
await page.goto(conditionSet.url);
await page.getByRole('button', { name: 'Edit Object' }).click();

// Add Condition to Condition Set
await page.getByRole('button', { name: 'Add Condition' }).click();

// Enter Condition Output
await page.getByLabel('Condition Name Input').first().fill('Negative');
await page
.locator('select[aria-label="Condition Output Type"]')
davetsay marked this conversation as resolved.
Show resolved Hide resolved
.first()
.selectOption({ value: 'string' });
await page.getByLabel('Condition Output String Input').first().fill('Negative');

// Condition Trigger default is okay so no change needed to form

// Enter Condition Criterion
await page
.locator('select[aria-label="Criterion Telemetry Selection"]')
davetsay marked this conversation as resolved.
Show resolved Hide resolved
.first()
.selectOption({ value: 'all' });
await page
.locator('select[aria-label="Criterion Metadata Selection"]')
davetsay marked this conversation as resolved.
Show resolved Hide resolved
.first()
.selectOption({ value: 'sin' });
await page
.locator('select[aria-label="Criterion Comparison Selection"]')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.locator('select[aria-label="Criterion Comparison Selection"]')
.getByLabel('Criterion Comparison Selection')

.first()
.selectOption({ value: 'lessThan' });
await page.getByLabel('Criterion Input').first().fill('0');

// Save the Condition Set
await page.getByRole('button', { name: 'Save' }).click();
await page.getByRole('listitem', { name: 'Save and Finish Editing' }).click();
});

test('You can remove telemetry from a condition set with existing conditions', async ({
page
}) => {
test.info().annotations.push({
type: 'issue',
description: 'https://github.com/nasa/openmct/issues/7710'
});

await page.getByLabel('Expand My Items folder').click();
await page.getByLabel(`Expand ${conditionSet.name} conditionSet`).click();
Comment on lines +530 to +531
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
await page.getByLabel('Expand My Items folder').click();
await page.getByLabel(`Expand ${conditionSet.name} conditionSet`).click();
// Reveal the currently navigated item in the tree
await page.getByLabel('Show selected item in tree');


await page
.getByLabel(`Navigate to ${exampleTelemetry.name}`, { exact: false })
.first()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make it specific enough to where we don't need the .first() ?

.click({ button: 'right' });

await page
.getByLabel(`${exampleTelemetry.name} Context Menu`)
.getByRole('menuitem', { name: 'Remove' })
.click();
await page.getByRole('button', { name: 'OK', exact: true }).click();

await page
.getByLabel(`Navigate to ${conditionSet.name} conditionSet Object`, { exact: true })
.click();
await page.getByRole('button', { name: 'Edit Object' }).click();
await page.getByRole('tab', { name: 'Elements' }).click();
expect(
await page
.getByRole('tabpanel', { name: 'Inspector Views' })
.getByRole('listitem', { name: exampleTelemetry.name })
.count()
).toEqual(0);
Comment on lines +548 to +553
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(
await page
.getByRole('tabpanel', { name: 'Inspector Views' })
.getByRole('listitem', { name: exampleTelemetry.name })
.count()
).toEqual(0);
expect(
await page
.getByRole('tabpanel', { name: 'Inspector Views' })
.getByRole('listitem', { name: exampleTelemetry.name })
).toHaveCount(0);

});
});
12 changes: 0 additions & 12 deletions src/plugins/condition/ConditionManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,6 @@ export default class ConditionManager extends EventEmitter {
applied: false
};
this.initialize();

this.stopObservingForChanges = this.openmct.objects.observe(
this.conditionSetDomainObject,
'*',
(newDomainObject) => {
this.conditionSetDomainObject = newDomainObject;
}
);
davetsay marked this conversation as resolved.
Show resolved Hide resolved
}

async requestLatestValue(endpoint) {
Expand Down Expand Up @@ -518,10 +510,6 @@ export default class ConditionManager extends EventEmitter {
Object.values(this.subscriptions).forEach((unsubscribe) => unsubscribe());
delete this.subscriptions;

if (this.stopObservingForChanges) {
this.stopObservingForChanges();
}

this.conditions.forEach((condition) => {
condition.destroy();
});
Expand Down
6 changes: 5 additions & 1 deletion src/plugins/condition/components/ConditionCollection.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@
-->

<template>
<section id="conditionCollection" :class="{ 'is-expanded': expanded }">
<section
id="conditionCollection"
:class="{ 'is-expanded': expanded }"
aria-label="Condition Set Condition Collection"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we using this label anywhere?

>
<div class="c-cs__header c-section__header">
<span
class="c-disclosure-triangle c-tree__item__view-control is-enabled"
Expand Down
9 changes: 8 additions & 1 deletion src/plugins/condition/components/ConditionItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
<div
class="c-condition-h"
:class="{ 'is-drag-target': draggingOver }"
aria-label="Condition Set Condition"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better to call it Condition Set Item ?

@dragover.prevent
@drop.prevent="dropCondition($event, conditionIndex)"
@dragenter="dragEnter($event, conditionIndex)"
Expand Down Expand Up @@ -83,6 +84,7 @@
<input
v-model="condition.configuration.name"
class="t-condition-input__name"
aria-label="Condition Name Input"
type="text"
@change="persist"
/>
Expand All @@ -91,7 +93,11 @@
<span class="c-cdef__label">Output</span>
<span class="c-cdef__controls">
<span class="c-cdef__control">
<select v-model="selectedOutputSelection" @change="setOutputValue">
<select
v-model="selectedOutputSelection"
aria-label="Condition Output Type"
@change="setOutputValue"
>
<option v-for="option in outputOptions" :key="option" :value="option">
{{ initCap(option) }}
</option>
Expand All @@ -101,6 +107,7 @@
<input
v-if="selectedOutputSelection === outputOptions[2]"
v-model="condition.configuration.output"
aria-label="Condition Output String Input"
davetsay marked this conversation as resolved.
Show resolved Hide resolved
class="t-condition-name-input"
type="text"
@change="persist"
Expand Down
7 changes: 5 additions & 2 deletions src/plugins/condition/components/ConditionSet.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,14 @@
-->

<template>
<div class="c-cs" :class="{ 'is-stale': isStale }">
<div class="c-cs" :class="{ 'is-stale': isStale }" aria-label="Condition Set">
<section class="c-cs__current-output c-section">
<div class="c-cs__content c-cs__current-output-value">
<span class="c-cs__current-output-value__label">Current Output</span>
<span class="c-cs__current-output-value__value" aria-label="Current Output Value">
<span
class="c-cs__current-output-value__value"
aria-label="Condition Set Current Output Value"
davetsay marked this conversation as resolved.
Show resolved Hide resolved
>
<template v-if="currentConditionOutput">
{{ currentConditionOutput }}
</template>
Expand Down
7 changes: 6 additions & 1 deletion src/plugins/condition/components/TestData.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@
-->

<template>
<section v-show="isEditing" id="test-data" :class="{ 'is-expanded': expanded }">
<section
v-show="isEditing"
id="test-data"
:class="{ 'is-expanded': expanded }"
aria-label="Condition Set Test Data"
>
<div class="c-cs__header c-section__header">
<span
class="c-disclosure-triangle c-tree__item__view-control is-enabled"
Expand Down
Loading