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

The FacilityDataset preset field is not used to reset facility defaults on UI #7334

Closed
jonboiser opened this issue Jul 20, 2020 · 1 comment · Fixed by #7733
Closed

The FacilityDataset preset field is not used to reset facility defaults on UI #7334

jonboiser opened this issue Jul 20, 2020 · 1 comment · Fixed by #7733
Assignees
Labels
P1 - important Priority: High impact on UX TAG: ux update Improved user-facing feature
Milestone

Comments

@jonboiser
Copy link
Contributor

jonboiser commented Jul 20, 2020

Observed behavior

Screenshot from https://kolibri-beta.learningequality.org/en/facility/#/a3f606630690237bbe94fe2a1a850af4/settings

Clicking "Reset to defaults" will always update the checkboxes to the default settings for "nonformal" facilities.

image

This will happen even if the FacilityDataset.preset field is not nonformal. This happens because we have hardcoded the defaults in this JS file:

https://github.com/learningequality/kolibri/blob/release-v0.14.x/kolibri/plugins/facility/assets/src/modules/facilityConfig/actions.js#L47-L59

Expected behavior

When a user clicks "reset to defaults", the defaults used are based on the FacilityDataset.preset field. And the end result are that the settings match what is this file. For example, if a facility was set up as an informal/personal device, then "Allow learner to edit username" will be checked, and so on, because the default value of learner_can_edit_username is true.

https://github.com/learningequality/kolibri/blob/release-v0.14.x/kolibri/core/auth/constants/facility_configuration_presets.json

User-facing consequences

Admins might not realize that a "preset" is actually chosen during the Setup Wizard and that these settings are related to that preset. Correcting the defaults after a reset would require knowing of the existence of the JSON file linked above and manually correct them.

Errors and logs

Steps to reproduce

  1. Set up a device with an informal/formal facility
  2. Change the settings on the settings page
  3. Click "reset to defaults"
  4. The new settings will not be the default for the facility preset, but will be for the nonformal preset.

Context

@jonboiser jonboiser added the TODO: needs clarification Insufficient information to proceed label Jul 20, 2020
@indirectlylit indirectlylit added the P1 - important Priority: High impact on UX label Jul 21, 2020
@indirectlylit indirectlylit added this to the upcoming patch milestone Jul 21, 2020
@jonboiser
Copy link
Contributor Author

jonboiser commented Oct 12, 2020

Currently, the way we handle the "reset" logic is by calling this Vuex action from the page

resetToDefaultSettings() {
this.showModal = false;
this.updateSettings('facilityConfig/resetFacilityConfig');
},
sendFacilityName(name) {

This works by updating the settings on the client using this hardcoded map, then sending that map back to the frontend.

export function resetFacilityConfig(store) {
store.commit('CONFIG_PAGE_MODIFY_ALL_SETTINGS', {
learner_can_edit_username: true,
learner_can_edit_name: true,
learner_can_edit_password: true,
learner_can_sign_up: true,
learner_can_delete_account: true,
learner_can_login_with_no_password: false,
show_download_button_in_learn: false,
});
return saveFacilityConfig(store);
}

However, this might be better handled on the backend by either introducing a new ViewSet or extending the current FacilityDatasetViewset to handle a "reset" operation, which requires no payload of specific settings, but would instead refer to the facility_configuration_presets.json file and update the settings based on that map. Then, the server would return the new map of settings (with the defaults), that the client could use to update itself.

Currently, the FacilityDatasetViewset might just allow the basic methods like PATCH, but maybe we can add another method-based endpoint to handle the reset operation

class FacilityDatasetViewSet(ValuesViewset):
permission_classes = (KolibriAuthPermissions,)
filter_backends = (KolibriAuthPermissionsFilter,)
serializer_class = FacilityDatasetSerializer
values = (
"id",
"learner_can_edit_username",
"learner_can_edit_name",
"learner_can_edit_password",
"learner_can_sign_up",
"learner_can_delete_account",
"learner_can_login_with_no_password",
"show_download_button_in_learn",
"description",
"location",
"registered",
"preset",
)
field_map = {"allow_guest_access": lambda x: allow_guest_access()}
def get_queryset(self):
queryset = FacilityDataset.objects.filter(
collection__kind=collection_kinds.FACILITY
)
facility_id = self.request.query_params.get("facility_id", None)
if facility_id is not None:
queryset = queryset.filter(collection__id=facility_id)
return queryset

@jonboiser jonboiser modified the milestones: upcoming patch, 0.14.5 Oct 19, 2020
@jonboiser jonboiser added the TAG: ux update Improved user-facing feature label Oct 19, 2020
@jonboiser jonboiser removed the TODO: needs clarification Insufficient information to proceed label Nov 6, 2020
@jonboiser jonboiser self-assigned this Jan 5, 2021
@jonboiser jonboiser modified the milestones: 0.14.6, 0.14.7 Jan 20, 2021
@jonboiser jonboiser modified the milestones: 0.14.8, 0.14.7 Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 - important Priority: High impact on UX TAG: ux update Improved user-facing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants