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

Update facility settings link display for Learn Only Device users #12123

Conversation

LianaHarris360
Copy link
Member

@LianaHarris360 LianaHarris360 commented May 1, 2024

Summary

Previously admin users on a learn-only device could see "You can also configure Facility settings” link in Device settings, but users on a LOD cannot access the facility settings page. This pull request modifies the Device Settings page to display the link if the user is not on a Learn Only Device.

  • Updates vuex mapGetters with isLearnerOnlyImport on the DeviceSettingsPage and modifies the conditional to display the KExternalLink that leads to FacilityConfigPage.

View of a super admin on a full device:

devicesettingspage

Updated view for admins on a learn only device:

Screenshot 2024-05-01 at 9 57 26 AM

References

Closes #11877

Reviewer guidance

  1. Follow the group learning setup and select Learn-Only device -> import or create a user account with an existing facility -> go to the Device settings page -> Verify that the message The changes you make here will affect this device only. is displayed.
  2. Follow the On My Own setup -> go to Device settings page -> Verify that the message and link The changes you make here will affect this device only. You can also configure device settings are displayed.

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@github-actions github-actions bot added APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) DEV: frontend labels May 1, 2024
@LianaHarris360 LianaHarris360 marked this pull request as ready for review May 1, 2024 15:52
Copy link
Member

@AllanOXDi AllanOXDi left a comment

Choose a reason for hiding this comment

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

Thanks for the changes @LianaHarris360 , Looks good!

@rtibbles
Copy link
Member

rtibbles commented May 3, 2024

@radinamatic can you confirm the fix manually here, and then we can merge?

@rtibbles
Copy link
Member

rtibbles commented May 3, 2024

Oh one thought here - as this doesn't involve any string updates, I think we could instead target this issue to the next 0.16 patch?

@LianaHarris360 LianaHarris360 changed the base branch from develop to release-v0.16.x May 3, 2024 18:30
@LianaHarris360 LianaHarris360 changed the base branch from release-v0.16.x to develop May 3, 2024 18:31
@LianaHarris360 LianaHarris360 force-pushed the update-facility-settings-link-display branch from 4800313 to c90e975 Compare May 3, 2024 18:52
@github-actions github-actions bot added DEV: dev-ops Continuous integration & deployment DEV: renderers HTML5 apps, videos, exercises, etc. DEV: backend Python, databases, networking, filesystem... APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: tools Internal tooling for development SIZE: very large labels May 3, 2024
@LianaHarris360 LianaHarris360 changed the base branch from develop to release-v0.16.x May 3, 2024 18:53
@LianaHarris360 LianaHarris360 force-pushed the update-facility-settings-link-display branch from c90e975 to 763cb26 Compare May 3, 2024 19:00
@LianaHarris360 LianaHarris360 removed DEV: dev-ops Continuous integration & deployment DEV: renderers HTML5 apps, videos, exercises, etc. DEV: backend Python, databases, networking, filesystem... APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: tools Internal tooling for development SIZE: very large labels May 3, 2024
@pcenov
Copy link
Member

pcenov commented May 7, 2024

Thanks @LianaHarris360 - I confirm that this is implemented as specified above, no issues observed while manually testing.

Copy link
Member

@radinamatic radinamatic left a comment

Choose a reason for hiding this comment

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

Manual QA passes, good to go! 👏🏽 💯 :shipit:

@LianaHarris360 LianaHarris360 merged commit f10889b into learningequality:release-v0.16.x May 7, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend SIZE: very small
Projects
None yet
5 participants