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

[Summary Widget] Can't add/edit rules #1858

Closed
VWoeltjen opened this Issue Dec 26, 2017 · 13 comments

Comments

Projects
None yet
5 participants
@VWoeltjen
Contributor

VWoeltjen commented Dec 26, 2017

Discovered during VISTA R3.4 testing; can't add rules or edit default displayed value/color of a summary widget. The relevant controls just don't appear.

image

@dtailor90

This comment has been minimized.

Show comment
Hide comment
@dtailor90

dtailor90 Dec 27, 2017

Contributor

Is this after adding telemetry?

Contributor

dtailor90 commented Dec 27, 2017

Is this after adding telemetry?

@charlesh88

This comment has been minimized.

Show comment
Hide comment
@charlesh88

charlesh88 Dec 27, 2017

Contributor

That was my question too. If you haven't added at least one telemetry, you should see a message indicating such - which I don't see in these screen shots.

Contributor

charlesh88 commented Dec 27, 2017

That was my question too. If you haven't added at least one telemetry, you should see a message indicating such - which I don't see in these screen shots.

@VWoeltjen

This comment has been minimized.

Show comment
Hide comment
@VWoeltjen

VWoeltjen Dec 27, 2017

Contributor

Yes, there was one telemetry object (a VISTA channel, more specifically) there at the time.

Contributor

VWoeltjen commented Dec 27, 2017

Yes, there was one telemetry object (a VISTA channel, more specifically) there at the time.

@VWoeltjen

This comment has been minimized.

Show comment
Hide comment
@VWoeltjen

VWoeltjen Dec 27, 2017

Contributor

After a brief skim it stands out that there are a lot of Zepto selectors involved in building the UI for the summary widget form; wonder if maybe these selectors don't all work under the snow theme?

Edit: Nope, snow works fine on openmct master.

Contributor

VWoeltjen commented Dec 27, 2017

After a brief skim it stands out that there are a lot of Zepto selectors involved in building the UI for the summary widget form; wonder if maybe these selectors don't all work under the snow theme?

Edit: Nope, snow works fine on openmct master.

@dtailor90

This comment has been minimized.

Show comment
Hide comment
@dtailor90

dtailor90 Dec 27, 2017

Contributor

hmm Interesting. I wonder wether dependencies are working properly? Any Error messages?

Contributor

dtailor90 commented Dec 27, 2017

hmm Interesting. I wonder wether dependencies are working properly? Any Error messages?

@VWoeltjen

This comment has been minimized.

Show comment
Hide comment
@VWoeltjen

VWoeltjen Dec 27, 2017

Contributor

No error messages on console. The message asking me to drop telemetry shows up when there's no telemetry, but once I add telemetry there is no Default Rule

Contributor

VWoeltjen commented Dec 27, 2017

No error messages on console. The message asking me to drop telemetry shows up when there's no telemetry, but once I add telemetry there is no Default Rule

@charlesh88

This comment has been minimized.

Show comment
Hide comment
@charlesh88

charlesh88 Dec 27, 2017

Contributor

I'm also seeing this on the uphill test server. When I add telem elems, I don't get any rules, and the telem elem doesn't appear in the Elements pool. If I save, I see the added telem elems contained in the widget in the tree.

Going in to edit the widget, and leaving the widget (either with Save or lose changes) does generate console errors: Uncaught (in promise).

Contributor

charlesh88 commented Dec 27, 2017

I'm also seeing this on the uphill test server. When I add telem elems, I don't get any rules, and the telem elem doesn't appear in the Elements pool. If I save, I see the added telem elems contained in the widget in the tree.

Going in to edit the widget, and leaving the widget (either with Save or lose changes) does generate console errors: Uncaught (in promise).

@VWoeltjen

This comment has been minimized.

Show comment
Hide comment
@VWoeltjen

VWoeltjen Dec 27, 2017

Contributor

Thanks for capturing that console error; I had filters turned on in Chrome without realizing it.

This line looks to be where the exception occurs. I believe there's a race condition where a composition map (compositionObjs) is expected to be populated before certain telemetry results come back. The problem is probably not reproducible with local storage (although may be able to reproduce it by adding an artificial delay)

Contributor

VWoeltjen commented Dec 27, 2017

Thanks for capturing that console error; I had filters turned on in Chrome without realizing it.

This line looks to be where the exception occurs. I believe there's a race condition where a composition map (compositionObjs) is expected to be populated before certain telemetry results come back. The problem is probably not reproducible with local storage (although may be able to reproduce it by adding an artificial delay)

@larkin

This comment has been minimized.

Show comment
Hide comment
@larkin

larkin Jan 2, 2018

Member

You can reproduce this with sine wave generators by changing GeneratorProvider's request method to:

    GeneratorProvider.prototype.request = function (domainObject, request) {
        return Promise.reject([]);
    };

The VISTA telemetry provider doing something unexpected: if it receives no results for a given query, it rejects the promise. I believe rejecting a promise the the correct way to handle erroneous responses, but "no data available" is not necessarily erroneous. Changing the VISTA provider to resolve the promise with an empty array would resolve this bug.

Member

larkin commented Jan 2, 2018

You can reproduce this with sine wave generators by changing GeneratorProvider's request method to:

    GeneratorProvider.prototype.request = function (domainObject, request) {
        return Promise.reject([]);
    };

The VISTA telemetry provider doing something unexpected: if it receives no results for a given query, it rejects the promise. I believe rejecting a promise the the correct way to handle erroneous responses, but "no data available" is not necessarily erroneous. Changing the VISTA provider to resolve the promise with an empty array would resolve this bug.

@larkin

This comment has been minimized.

Show comment
Hide comment
@larkin

larkin Jan 2, 2018

Member

The rejection mentioned above does cause the error message that Charles logged, but that's not the root cause.

The SummaryWidget uses the status capability to determine if the object is being edited, and it seems there is a bug with the status capability and object namespaces. The statusService has an entry for "<object-prefix>:<object-id>", but the status capability uses domainObject.getId() which returns only "<object-id>" I'm not exactly sure how it gets to this state, but this is what I see:

screen shot 2018-01-02 at 3 37 44 pm

As a result, summeryWidget.editing is set to false, and the rule section gets hidden.

I think the solution here that would be inline (for now) with our other displays is to use the editorCapability to check if the object is being edited instead of the status listener. And figure out why the status listener isn't working.

Member

larkin commented Jan 2, 2018

The rejection mentioned above does cause the error message that Charles logged, but that's not the root cause.

The SummaryWidget uses the status capability to determine if the object is being edited, and it seems there is a bug with the status capability and object namespaces. The statusService has an entry for "<object-prefix>:<object-id>", but the status capability uses domainObject.getId() which returns only "<object-id>" I'm not exactly sure how it gets to this state, but this is what I see:

screen shot 2018-01-02 at 3 37 44 pm

As a result, summeryWidget.editing is set to false, and the rule section gets hidden.

I think the solution here that would be inline (for now) with our other displays is to use the editorCapability to check if the object is being edited instead of the status listener. And figure out why the status listener isn't working.

@larkin

This comment has been minimized.

Show comment
Hide comment
@larkin

larkin Jan 2, 2018

Member

Ok, digging more, it's because the summary widget doesn't properly translate the new-style identifier to an old-style identifier before querying the object API. Quick candidate fix incoming.

Member

larkin commented Jan 2, 2018

Ok, digging more, it's because the summary widget doesn't properly translate the new-style identifier to an old-style identifier before querying the object API. Quick candidate fix incoming.

larkin added a commit that referenced this issue Jan 2, 2018

[SummaryWidget] Use objectutil to get legacy id
Use objectUtils to get a proper legacy id so that namespaces are
properly handled.  Fixes #1858

larkin added a commit that referenced this issue Jan 2, 2018

merge summary widget namespace fix
commit a04fba5
Author: Pete Richards <peter.l.richards@nasa.gov>
Date:   Tue Jan 2 16:05:13 2018 -0500

    Add test for identifier generation

commit 77fa775
Author: Pete Richards <peter.l.richards@nasa.gov>
Date:   Tue Jan 2 15:56:34 2018 -0500

    [SummaryWidget] Use objectutil to get legacy id

    Use objectUtils to get a proper legacy id so that namespaces are
    properly handled.  Fixes #1858
@VWoeltjen

This comment has been minimized.

Show comment
Hide comment
@VWoeltjen

VWoeltjen Jan 3, 2018

Contributor

Verified fixed, able to add telemetry and edit the summary widget rules thereafter with this change.

VISTA

  • Version 3.4.0-prerelease-2
  • Revision 5e3aefdbee504fd74547005c432925ee7786762f
  • Built 2018-01-02 17:40:34.851-0500
Contributor

VWoeltjen commented Jan 3, 2018

Verified fixed, able to add telemetry and edit the summary widget rules thereafter with this change.

VISTA

  • Version 3.4.0-prerelease-2
  • Revision 5e3aefdbee504fd74547005c432925ee7786762f
  • Built 2018-01-02 17:40:34.851-0500

@VWoeltjen VWoeltjen closed this Jan 3, 2018

VWoeltjen added a commit that referenced this issue Jan 3, 2018

[SummaryWidget] Use objectutil to get legacy id
Use objectUtils to get a proper legacy id so that namespaces are
properly handled.  Fixes #1858

@akhenry akhenry added the unverified label Feb 20, 2018

@charlesh88

This comment has been minimized.

Show comment
Hide comment
@charlesh88

charlesh88 Feb 21, 2018

Contributor

Testathon 2/21/2018: Verified fixed

Contributor

charlesh88 commented Feb 21, 2018

Testathon 2/21/2018: Verified fixed

@akhenry akhenry removed the unverified label Feb 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment