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

Adapt kolibri to use latest version of xstate library #12783

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion kolibri/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@
"vue-router": "^3.6.5",
"vuex": "^3.6.2",
"vuex-router-sync": "^5.0.0",
"xstate": "^4.38.3"
"xstate": "^5.18.2"
}
}
205 changes: 81 additions & 124 deletions kolibri/plugins/setup_wizard/assets/src/machines/wizardMachine.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,18 @@ import { DeviceTypePresets, FacilityTypePresets, LodTypePresets, UsePresets } fr
*
* However, to truly emulate the machine, you will need to use the `Events` tab. If you are ever
* on a node that says `DO/` then that transition expects an event to be sent with a `value`
* property. You can use the "Send Event" button to open a little dialog where you can enter
* property. You can use the "raise Event" button to open a little dialog where you can enter
* JSON. One downside here is that you'll have to reference the *value* of the constants as you
* cannot reference the code in this dialog.
*
* You can click on events to send them, but if there is a `DO/` it implies actions with
* You can click on events to raise them, but if there is a `DO/` it implies actions with
* side effects will occur. Those actions will take in the whole JSON object that you send
* in the `Events` tab. Note that the use of `value` as a property is just a preference and
* the only required property is the `type` property indicating what kind of event is sent.
*/

/* eslint-disable-next-line */
import { assign, createMachine } from 'xstate';
import { createMachine, assign } from 'xstate';

// NOTE: Uncomment the following function if you're using the visualizer
// const checkCapability = capabilityToCheck => ["get_os_user"].includes(capabilityToCheck);
Expand Down Expand Up @@ -77,7 +77,6 @@ export const wizardMachine = createMachine(
id: 'wizard',
initial: 'initializeContext',
context: initialContext,
predictableActionArguments: true,
on: {
START_OVER: { target: 'howAreYouUsingKolibri', actions: 'resetContext' },
},
Expand All @@ -103,11 +102,11 @@ export const wizardMachine = createMachine(
{
// `cond` takes a function that returns a Boolean, continuing to the
Copy link
Member

Choose a reason for hiding this comment

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

Picking a nit - the comment here still references cond.

// `target` when it returns truthy
cond: 'isOnMyOwnOrGroup',
guard: 'isOnMyOwnOrGroup',
target: 'defaultLanguage',
},
{
cond: 'isGroupSetup',
guard: 'isGroupSetup',
target: 'deviceName',
},
// There is no fallback path here; if neither `cond` above is truthy, this will break
Expand All @@ -127,7 +126,7 @@ export const wizardMachine = createMachine(
on: { BACK: 'defaultLanguage' },
always: [
{
cond: 'canGetOsUser',
guard: 'canGetOsUser',
target: 'finalizeSetup',
},
{
Expand Down Expand Up @@ -167,11 +166,11 @@ export const wizardMachine = createMachine(
on: { BACK: 'fullOrLearnOnlyDevice' },
always: [
{
cond: 'isLodSetup',
guard: 'isLodSetup',
target: 'importLodUsers',
},
{
cond: 'isFullSetup',
guard: 'isFullSetup',
target: 'fullDeviceNewOrImportFacility',
},
],
Expand All @@ -191,11 +190,11 @@ export const wizardMachine = createMachine(
on: { BACK: 'fullDeviceNewOrImportFacility' },
always: [
{
cond: 'isNewFacility',
guard: 'isNewFacility',
target: 'setFacilityPermissions',
},
{
cond: 'isImportFacility',
guard: 'isImportFacility',
target: 'importFacility',
},
],
Expand Down Expand Up @@ -343,7 +342,7 @@ export const wizardMachine = createMachine(
lodProceedJoinOrNew: {
always: [
{
cond: ctx => ctx.lodImportOrJoin === LodTypePresets.JOIN,
guard: ctx => ctx.lodImportOrJoin === LodTypePresets.JOIN,
target: 'lodJoinFacility',
},
{
Expand Down Expand Up @@ -424,45 +423,33 @@ export const wizardMachine = createMachine(
{
actions: {
// The `assign` function takes an object that maps keys that match those in the machine's
// `context`to functions that take two parameters `(context, event)` - where the context
// `context`to functions that take two parameters `({context, event})` - where the context
// is the current context and event refers to the event sent to the machine to initiate a
// transition.
setOnMyOwnOrGroup: assign({
onMyOwnOrGroup: (_, event) => event.value,
}),
setDeviceName: assign({
deviceName: (_, event) => event.value,
}),
setFullOrLOD: assign({
fullOrLOD: (_, event) => event.value,
}),
setCanGetOsUser: assign({
canGetOsUser: (_, event) => event.value,
}),
setFacilityNewOrImport: assign({
facilityNewOrImport: (_, event) => {
return event.value.importOrNew;
},
importDeviceId: (_, event) => {
return event.value.importDeviceId;
},
}),
setSuperuser: assign({
superuser: (_, event) => {
return event.value;
},
}),
setSelectedImportDeviceFacility: assign({
selectedFacility: (_, event) => {
return event.value.selectedFacility;
},
importDevice: (_, event) => {
return event.value.importDevice;
},
facilitiesOnDeviceCount: (_, event) => {
return event.value.facilitiesCount;
},
}),
setOnMyOwnOrGroup: assign((_, event) => ({
onMyOwnOrGroup: event.value,
})),
setDeviceName: assign((_, event) => ({
deviceName: event.value,
})),
setFullOrLOD: assign((_, event) => ({
fullOrLOD: event.value,
})),
setCanGetOsUser: assign((_, event) => ({
canGetOsUser: event.value,
})),
setFacilityNewOrImport: assign((_, event) => ({
facilityNewOrImport: event.value.importOrNew,
importDeviceId: event.value.importDeviceId,
})),
setSuperuser: assign((_, event) => ({
superuser: event.value,
})),
setSelectedImportDeviceFacility: assign((_, event) => ({
selectedFacility: event.value.selectedFacility,
importDevice: event.value.importDevice,
facilitiesOnDeviceCount: event.value.facilitiesCount,
})),
clearSelectedSetupType: assign({
facilityNewOrImport: () => null,
}),
Expand All @@ -475,97 +462,67 @@ export const wizardMachine = createMachine(
selectedFacility: () => null,
importDeviceId: () => null,
}),
setFacilityTypeAndName: assign({
formalOrNonformal: (_, event) => event.value.selected,
facilityName: (_, event) => event.value.facilityName,
}),
setGuestAccess: assign({
guestAccess: (_, event) => event.value,
}),
setLearnerCanCreateAccount: assign({
learnerCanCreateAccount: (_, event) => event.value,
}),
setRequirePassword: assign({
requirePassword: (_, event) => event.value,
}),
setLodType: assign({
lodImportOrJoin: (_, event) => event.value.importOrJoin,
importDeviceId: (_, event) => event.value.importDeviceId,
}),
setLodImportDeviceId: assign({
importDeviceId: (_, event) => event.value,
}),
addImportedUser: assign({
importedUsers: (ctx, event) => {
const users = ctx.importedUsers;
users.push(event.value);
return uniq(users);
},
}),
setFirstLodUser: assign({
firstImportedLodUser: (_, event) => event.value,
}),
setLodAdmin: assign({
// Used when setting the Admin user for multiple import
lodAdmin: (_, event) => {
return {
username: event.value.adminUsername,
password: event.value.adminPassword,
id: event.value.id,
};
setFacilityTypeAndName: assign((_, event) => ({
formalOrNonformal: event.value.selected,
facilityName: event.value.facilityName,
})),
setGuestAccess: assign((_, event) => ({
guestAccess: event.value,
})),
setLearnerCanCreateAccount: assign((_, event) => ({
learnerCanCreateAccount: event.value,
})),
setRequirePassword: assign((_, event) => ({
requirePassword: event.value,
})),
setLodType: assign((_, event) => ({
lodImportOrJoin: event.value.importOrJoin,
importDeviceId: event.value.importDeviceId,
})),
addImportedUser: assign((ctx, event) => ({
importedUsers: uniq([...ctx.importedUsers, event.value]),
})),
setFirstLodUser: assign((_, event) => ({
firstImportedLodUser: event.value,
})),
setLodAdmin: assign((_, event) => ({
lodAdmin: {
username: event.value.adminUsername,
password: event.value.adminPassword,
id: event.value.id,
},
}),
setLodSuperAdmin: assign({
})),
setLodSuperAdmin: assign((ctx, event) => ({
// Sets the super admin to be set as the device super admin -- the first LOD user imported
superuser: (ctx, event) => {
if (!ctx.superuser) {
return {
username: event.value.username,
password: event.value.password,
};
} else {
return ctx.superuser;
}
superuser: ctx.superuser || {
username: event.value.username,
password: event.value.password,
},
}),
setRemoteUsers: assign({
remoteUsers: (_, event) => event.value.users,
}),
})),
setRemoteUsers: assign((_, event) => ({
remoteUsers: event.value.users,
})),
/**
* Assigns the machine to have the initial context again while maintaining the value of
* canGetOsUser.

* This effectively resets the machine's state
*/
resetContext: assign(initialContext),
resetContext: assign(() => initialContext),
setImportedFacility: assign({
isImportedFacility: () => {
return true;
},
isImportedFacility: () => true,
}),
},
guards: {
// Functions used to return a true/false value. When the functions are called, they are passed
// the current value of the machine's context as the only parameter
isOnMyOwnOrGroup: context => {
return context.onMyOwnOrGroup === Presets.PERSONAL;
},
isGroupSetup: context => {
return context.onMyOwnOrGroup === UsePresets.GROUP;
},
isOnMyOwnOrGroup: context => context.onMyOwnOrGroup === Presets.PERSONAL,
isGroupSetup: context => context.onMyOwnOrGroup === UsePresets.GROUP,
canGetOsUser: () => checkCapability('get_os_user'),
isNewFacility: context => {
return context.facilityNewOrImport === FacilityTypePresets.NEW;
},
isImportFacility: context => {
return context.facilityNewOrImport === FacilityTypePresets.IMPORT;
},
isLodSetup: context => {
return context.fullOrLOD === DeviceTypePresets.LOD;
},
isFullSetup: context => {
return context.fullOrLOD === DeviceTypePresets.FULL;
},
isNewFacility: context => context.facilityNewOrImport === FacilityTypePresets.NEW,
isImportFacility: context => context.facilityNewOrImport === FacilityTypePresets.IMPORT,
Copy link
Member

Choose a reason for hiding this comment

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

I notice that the function passed to guard seems to be expecting an object on the first param (relevant docs) shaped like { context, event } -- but here we only handle one param as if it's the context.

Makes me wonder if there might be issues uncovered by tests in the setup wizard because of this.

isLodSetup: context => context.fullOrLOD === DeviceTypePresets.LOD,
isFullSetup: context => context.fullOrLOD === DeviceTypePresets.FULL,
},
},
);
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

<script>

import { interpret } from 'xstate';
import { createActor } from 'xstate';
import { mapState } from 'vuex';
import commonCoreStrings from 'kolibri.coreVue.mixins.commonCoreStrings';
import useKResponsiveWindow from 'kolibri-design-system/lib/composables/useKResponsiveWindow';
Expand Down Expand Up @@ -51,7 +51,7 @@
},
data() {
return {
service: interpret(wizardMachine),
service: createActor(wizardMachine),
};
},
provide() {
Expand Down Expand Up @@ -117,7 +117,7 @@

this.service.start(savedState);

this.service.onTransition(state => {
this.service.subscribe(state => {
synchronizeRouteAndMachine(state);
Lockr.set('savedState', this.service._state);
});
Expand Down
Loading
Loading