KDS to Studio: Use KModal in policy modals by MisRob · Pull Request #3127 · learningequality/studio

MisRob · 2021-04-29T14:11:53Z

Summary

Description of the change(s) you made

Use KModal in policy modals - Terms of Service, Privacy Policy, and Community Standards. It also contains simplifications of policies store tests and updates to policies modal components architecture that have made KDS refactor and updating/adding tests easier. See commit messages for detailed explanations.

Manual verification steps performed

Preview policies on login page

Navigate to http://127.0.0.1:8080/en/accounts/#/

2. Click the "Privacy policy" link at the bottom of the page and see the privacy policy modal appear
3. Click the "Terms of service" link at the bottom of the page and see the terms of service modal appear

Preview policies on registration page

Navigate to http://127.0.0.1:8080/en/accounts/#/create

2. Click the "View terms of service" link at the bottom of the form and see the terms of service modal appear
3. Click the "View privacy policy" link at the bottom of the form and see the privacy policy modal appear

Accept a policy after a policy update

Log in
Emulate policies updates by changing dates here, for example to:

export const policyDates = {
  [policies.PRIVACY]: new Date('2021-01-10'),
  [policies.TERMS_OF_SERVICE]: new Date('2021-01-10'),
  [policies.COMMUNITY_STANDARDS]: new Date('2021-01-30'),
};

(you may need to reload a page if reloading wasn't triggered by the development server restart after the code updates)

4. Make sure that you can see "Terms of service" and "Privacy policy" immediately after logging in and need to accept it before Studio allows you to proceed. Make sure that you can't navigate away from policies modals without accepting updated policies. Make sure that "Field is required" error displays when you don't accept a policy and click "Continue".

Preview policies in "About Studio" tab in "Settings"

Log in
Navigate to http://127.0.0.1:8080/en/settings/#/using-studio and in "Kolibri Studio resources" section at the top of the page

3. Click the "Privacy policy" link and see the privacy policy modal appear
4. Click the "Terms of service" link and see the terms of service modal appear
5. Click the "Community standards" link and see the community standards modal appear

Important modals interactions

Open the community standards modal from the terms of service modal by navigating to section "8. Community Standards" on the terms of service modal and clicking "Learn more about Studio's community standards" link
Open the privacy policy modal from the terms of service modal by navigating to section "9. Your Privacy" on the terms of service modal and clicking "Learn more about Studio's privacy policy" link
Open the terms of service modal from the privacy policy modal by clicking "Terms and Conditions" link in the introduction section of the privacy policy modal
Check that you can close all three modals by clicking "Close" button

Screenshots

	Before	After
Terms of Service
Privacy Policy
Community Standards
Updated Terms of Service
Updated Privacy Policy
Error

Does this introduce any tech-debt items?

It rather reduces it.

One thing to discuss is whether we want to support error state for KCheckbox (see the "Error" before/after screenshots) to

bring back the red checkbox outline (it has been lost in this PR)
have an error message as part of the KCheckbox component (it is implemented outside of it in this PR)

I haven't seen many similar use cases though so maybe there won't be any further updates needed. I'd like to discuss this with KDS team and also chat about better error message instead of generic "Field is required" (e.g "You need to agree to Terms and Conditions") on that opportunity. If there are some updates needed, I will open follow-up KDS/Studio issues.

Reviewer guidance

How can a reviewer test these changes?

Please follow "Manual verification steps"

Are there any risky areas that deserve extra testing?

Policies are important part of the app and during components architecture simplifications I updated core parts of policies logic so please follow test steps thouroughly, especially "Accept a policy after a policy update"

References

Closes learningequality/kolibri-design-system#157
Closes learningequality/kolibri-design-system#159

Comments

I've added "user strings" tag. Although there are no new strings introduced, I moved some between components which will change their namespace and might require @radinamatic's approval on Crowdin.

Contributor's Checklist

PR process:

If this is an important user-facing change, PR or related issue the CHANGELOG label been added to this PR. Note: items with this label will be added to the CHANGELOG at a later time
If this includes an internal dependency change, a link to the diff is provided
The docs label has been added if this introduces a change that needs to be updated in the user docs?
If any Python requirements have changed, the updated requirements.txt files also included in this PR
Opportunities for using Google Analytics here are noted
Migrations are safe for a large db

Studio-specifc:

All user-facing strings are translated properly
The notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)
All UI components are LTR and RTL compliant
Views are organized into pages, components, and layouts directories as described in the docs
Users' storage used is recalculated properly on any changes to main tree files
If there new ways this uses user data that needs to be factored into our Privacy Policy, it has been noted.

Testing:

Code is clean and well-commented
Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Any new interactions have been added to the QA Sheet
Critical and brittle code paths are covered by unit tests

Reviewer's Checklist

This section is for reviewers to fill out.

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

- get rid of `[vuex] unknown mutation type: policies/SET_POLICIES` error by importing the whole policies module and then overriding only values that are needed for a test - remove tests that were prone to giving false positives and weren't testing responsibilities of PolicyModals component

- user props and events in simpler modal components (PoliciesModal, TermsOfServiceModal, PrivacyPolicyModal, CommunityStandardsModal) to decouple them from store to make logic simpler and easier to test - related tests simplifications of PoliciesModal - related updates of store and PolicyModals component that is now the only controller component encapsulating all Vuex logic - some more tests for PoliciesModal nad PolicyModals

- export getters, mutations, and actions to allow testing them directly instead of the need to initialize the whole store, similarly to how we test getters etc. of our other store modules (this has also allowed deletion of mocks that are not related to this test suite) - use hardcoded sample data in tests to improve clarity and make orientation in tests faster when reading them as documentation - add more documentation strings to store functions - move the test for updating `window.user` object to plugin.spec.js to make sure that `window.user` will be updated every time `setPolicies` mutation is called, and not only when `setPolicies` is called from `acceptPolicy` action

`KModal`'s content slot is already wrapped in form => leaving `VForm` here would result in nested forms, which is not supported and could result in bugs (see https://stackoverflow.com/questions/379610/can-you-nest-html-forms). - remove `VForm` - replace `Checkbox` with `KCheckbox` (related to `VForm` deletion in terms of validation logic) - update validation logic to resemble `VForm`'s behaviour but to work with `KCheckbox` and native `<form>` in `KModal`

Because `PolicyModals` was in both `UsingStudio` and `SettingsIndex` components, it was open two times which also caused a focus bug. This removes `PolicyModals` component from `UsingStudio` and leaves it only in `SettingsIndex`.

marcellamaki

This looks great! Thank you (as always) for such a thorough PR write up 😄

indirectlylit · 2021-05-19T18:14:27Z

This looks awesome, great job!

One very minor note is there's a bit of extra whitespace in the bottom bar below checkboxes:

indirectlylit · 2021-05-19T18:50:53Z

bring back the red checkbox outline (it has been lost in this PR)

I personally prefer it without the red border

MisRob added 2 commits May 12, 2021 08:26

Replace ResponsiveDialog by KModal in PoliciesModal

7c2538b

MisRob force-pushed the kds-kmodal-responsivedialog-policiesmodal branch 4 times, most recently from a744954 to dcc4ca5 Compare May 12, 2021 07:23

MisRob force-pushed the kds-kmodal-responsivedialog-policiesmodal branch 2 times, most recently from ebe86ed to 5d0a20f Compare May 12, 2021 14:19

MisRob force-pushed the kds-kmodal-responsivedialog-policiesmodal branch from 5d0a20f to 6a232af Compare May 12, 2021 14:23

MisRob added 5 commits May 12, 2021 17:00

Add tests for a new getter and fix bugs

9dc0bd8

Emit accept and close events

c8301f2

Remove typo

89e6e73

Make policies accessible from template

2a23432

Fix typo

d6fd651

MisRob force-pushed the kds-kmodal-responsivedialog-policiesmodal branch from f5f8b57 to 626ccb8 Compare May 13, 2021 10:55

MisRob force-pushed the kds-kmodal-responsivedialog-policiesmodal branch from 626ccb8 to 6ae90c7 Compare May 13, 2021 10:56

MisRob added 2 commits May 14, 2021 11:53

Remove duplicate policy modals

4099853

Because `PolicyModals` was in both `UsingStudio` and `SettingsIndex` components, it was open two times which also caused a focus bug. This removes `PolicyModals` component from `UsingStudio` and leaves it only in `SettingsIndex`.

Remove ResponsiveDialog component

f633f83

MisRob added TAG: user strings TODO: needs review DEV: frontend labels May 14, 2021

MisRob marked this pull request as ready for review May 14, 2021 12:06

MisRob requested a review from marcellamaki May 17, 2021 06:24

marcellamaki approved these changes May 17, 2021

View reviewed changes

MisRob merged commit 2b931b7 into learningequality:unstable May 17, 2021

bjester mentioned this pull request May 25, 2021

Hotfixes release for sprint ending 2021-05-24 #3166

Merged

MisRob deleted the kds-kmodal-responsivedialog-policiesmodal branch September 15, 2021 12:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

KDS to Studio: Use KModal in policy modals#3127

KDS to Studio: Use KModal in policy modals#3127
MisRob merged 12 commits intolearningequality:unstablefrom
MisRob:kds-kmodal-responsivedialog-policiesmodal

MisRob commented Apr 29, 2021 •

edited

Loading

Uh oh!

marcellamaki left a comment

Uh oh!

indirectlylit commented May 19, 2021

Uh oh!

indirectlylit commented May 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

MisRob commented Apr 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Description of the change(s) you made

Manual verification steps performed

Preview policies on login page

Preview policies on registration page

Accept a policy after a policy update

Preview policies in "About Studio" tab in "Settings"

Important modals interactions

Screenshots

Does this introduce any tech-debt items?

Reviewer guidance

How can a reviewer test these changes?

Are there any risky areas that deserve extra testing?

References

Comments

Contributor's Checklist

Reviewer's Checklist

This section is for reviewers to fill out.

Uh oh!

marcellamaki left a comment

Choose a reason for hiding this comment

Uh oh!

indirectlylit commented May 19, 2021

Uh oh!

indirectlylit commented May 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MisRob commented Apr 29, 2021 •

edited

Loading