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

task/WP-315: Handle interactive session messages #877

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

rstijerina
Copy link
Member

Overview

Some apps may require additional, freeform information to be rendered to the interactive session modal. This PR takes the "message" param from a webhook if it exists and renders it to the user.

Related

Testing

  1. Update your settings_secret.py with https://stache.utexas.edu/entry/bedc97190d3a907cb44488785440595c
  2. Uncomment the lines under "DEV TENANT SETTINGS" to enable the dev tenant locally
  3. Sign out and back in to https://cep.test/login
  4. Run ./manage.py import-apps -c in your local core_portal_django container to get the latest apps
  5. Run an Rstudio session at https://cep.test/workbench/applications/rstudio?appVersion=4.3
  6. Confirm the interactive session modal includes a message with a session password

UI

Screenshot 2023-10-06 at 3 55 39 PM

Notes

Inviting feedback on how this should look.

@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #877 (799c408) into main (982c7d6) will decrease coverage by 0.04%.
The diff coverage is 29.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #877      +/-   ##
==========================================
- Coverage   62.99%   62.95%   -0.04%     
==========================================
  Files         427      427              
  Lines       12197    12211      +14     
  Branches     2506     2509       +3     
==========================================
+ Hits         7683     7687       +4     
- Misses       4306     4315       +9     
- Partials      208      209       +1     
Flag Coverage Δ
javascript 68.69% <100.00%> (+0.01%) ⬆️
unittests 57.20% <7.69%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ponents/Jobs/JobsSessionModal/JobsSessionModal.jsx 100.00% <100.00%> (ø)
...ient/src/components/Jobs/JobsStatus/JobsStatus.jsx 88.23% <100.00%> (+0.23%) ⬆️
server/portal/apps/webhooks/views.py 90.00% <33.33%> (-1.76%) ⬇️
...workspace_operations/shared_workspace_migration.py 0.00% <0.00%> (ø)

Copy link
Member

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

If feasible—without novel customization and without affecting other modals—have modal window by wide enough to fit all the text (like in the design).

UI matches design sans bold login statement. If there is no need to login, then this looks fine.

Copy link
Collaborator

@chandra-tacc chandra-tacc left a comment

Choose a reason for hiding this comment

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

works great and LGTM.

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.

3 participants