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

Operator status #5179

Merged
merged 59 commits into from
Jun 2, 2022
Merged

Operator status #5179

merged 59 commits into from
Jun 2, 2022

Conversation

charlesh88
Copy link
Contributor

@charlesh88 charlesh88 commented May 9, 2022

Closes #5193

Describe your changes:

Operator status functionality for managing and responding to operations status polls questions.

To Do

@charlesh88

@akhenry

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

  • Changes address original issue?
  • Unit tests included and/or updated with changes?
  • Command line build passes?
  • Has this been smoke tested?
  • Testing instructions included in associated issue?

Reviewer Checklist

  • Changes appear to address issue?
  • Changes appear not to be breaking changes?
  • Appropriate unit tests included?
  • Code style and in-line documentation are appropriate?
  • Commit messages meet standards?
  • Has associated issue been labelled unverified? (only applicable if this PR closes the issue)
  • Has associated issue been labelled bug? (only applicable if this PR is for a bug fix)

@charlesh88
Copy link
Contributor Author

charlesh88 commented May 9, 2022

Todos

@akhenry

  • Rather than always using a predetermined CSS class, allow indicators to optionally set their icon and its color image
    via a configured color property (like this in ExampleUserProvider.js):
const STATUSES = [{
    key: "GO",
    label: "GO",
    iconClass: "icon-check",
    iconClassPoll: "icon-status-poll-check",
    statusClass: "s-status-ok",
    statusBgColor: "#33cc33",
    statusFgColor: "#000"
}, {

akhenry and others added 4 commits May 10, 2022 07:48
- Changed user indicator to display response when set to other than "NO_STATUS".
- Standardized icon display.
… is the most full and complete version of the symbols font - OVERRIDE ANY MERGE CONFLICTS WITH THIS COMMIT!
@akhenry
Copy link
Contributor

akhenry commented May 18, 2022

Implementation Notes

  • This implements the API for supporting operator status, and extends the existing Example User plugin to handle status.
  • The existing User API has been extended with new status-specific functionality via a sub-api that is referenced like so:
    openmct.user.status.whatever
  • There are two classes that constitute interfaces and have only been included as it was the simplest way of documenting these interfaces. They are UserProvider and StatusUserProvider
  • I believe this is the first usage of private class members in Open MCT, which will break downstream projects if they are compiling Open MCT into their build artifacts and not using Webpack 5.
  • A couple of concepts may not be immediately clear just from looking at the code:
    • A "status role" is a user role that can provide status updates. The User API already had basic support for roles, but not all user roles will necessarily be providing status. This is because not every user of Open MCT during a mission is necessarily involved in real-time operations, and so an operational status is not needed from them. As such, the API distinguishes between regular user roles and "status roles"
  • To enable the example user plugin and test Operator Status locally, add the following lines to index.html:
openmct.install(openmct.plugins.example.ExampleUser({
    autoLoginUser: 'guest',
    defaultStatusRole: 'Driver'
}));
openmct.install(openmct.plugins.OperatorStatus());

@codecov
Copy link

codecov bot commented May 18, 2022

Codecov Report

Merging #5179 (e7323f8) into master (50b642f) will decrease coverage by 0.12%.
The diff coverage is 31.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5179      +/-   ##
==========================================
- Coverage   50.28%   50.15%   -0.13%     
==========================================
  Files         548      556       +8     
  Lines       20133    20393     +260     
  Branches     1866     1876      +10     
==========================================
+ Hits        10123    10229     +106     
- Misses       9514     9677     +163     
+ Partials      496      487       -9     
Impacted Files Coverage Δ
src/api/api.js 100.00% <ø> (ø)
src/api/telemetry/TelemetryMetadataManager.js 70.21% <0.00%> (ø)
...plugins/displayLayout/components/TelemetryView.vue 2.80% <ø> (ø)
src/plugins/objectMigration/Migrations.js 1.61% <0.00%> (ø)
.../plugins/operatorStatus/AbstractStatusIndicator.js 0.00% <0.00%> (ø)
...s/operatorStatus/operatorStatus/OperatorStatus.vue 0.00% <0.00%> (ø)
...orStatus/operatorStatus/OperatorStatusIndicator.js 0.00% <0.00%> (ø)
src/plugins/operatorStatus/plugin.js 0.00% <0.00%> (ø)
...ugins/operatorStatus/pollQuestion/PollQuestion.vue 0.00% <0.00%> (ø)
...eratorStatus/pollQuestion/PollQuestionIndicator.js 0.00% <0.00%> (ø)
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50b642f...e7323f8. Read the comment docs.

@akhenry akhenry marked this pull request as ready for review May 18, 2022 16:31
@akhenry akhenry requested review from scottbell and shefalijoshi and removed request for scottbell May 18, 2022 16:31
Copy link
Contributor

@shefalijoshi shefalijoshi left a comment

Choose a reason for hiding this comment

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

This is really solid! Nicely done!

I'm curious, what is the expected behavior if the same user is logged in on multiple tabs?

src/api/user/StatusAPI.js Outdated Show resolved Hide resolved
src/api/user/StatusAPI.js Show resolved Hide resolved
@shefalijoshi shefalijoshi self-requested a review June 2, 2022 20:42
Copy link
Contributor

@shefalijoshi shefalijoshi left a comment

Choose a reason for hiding this comment

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

Looks good to me

@shefalijoshi shefalijoshi merged commit f5796c9 into master Jun 2, 2022
@shefalijoshi shefalijoshi deleted the operator-status branch June 2, 2022 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Operator Status Indicators
3 participants