-
Notifications
You must be signed in to change notification settings - Fork 699
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
added functionality to create new facility on existing kolibri #11197
Changes from 5 commits
c85df81
b39eac9
6d36f25
bced591
df5bdb1
00a6031
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,15 @@ | |
from __future__ import print_function | ||
from __future__ import unicode_literals | ||
|
||
import logging | ||
|
||
from django.core.validators import MinLengthValidator | ||
from django.db import transaction | ||
from rest_framework import serializers | ||
from rest_framework.exceptions import ParseError | ||
from rest_framework.validators import UniqueTogetherValidator | ||
|
||
from .constants import facility_presets | ||
from .errors import IncompatibleDeviceSettingError | ||
from .errors import InvalidCollectionHierarchy | ||
from .errors import InvalidMembershipError | ||
|
@@ -20,6 +25,9 @@ | |
from kolibri.core.auth.constants.demographics import NOT_SPECIFIED | ||
|
||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class RoleSerializer(serializers.ModelSerializer): | ||
class Meta: | ||
model = Role | ||
|
@@ -137,6 +145,27 @@ class Meta: | |
fields = ("id", "name") | ||
|
||
|
||
class CreateFacilitySerializer(serializers.ModelSerializer): | ||
preset = serializers.ChoiceField(choices=facility_presets.choices) | ||
|
||
class Meta: | ||
model = Facility | ||
fields = ("id", "name", "preset") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are taking the preset here, but not doing anything with it. See here for an example of how the preset data is used in a serializer in order to set the settings appropriately: https://github.com/learningequality/kolibri/blob/release-v0.16.x/kolibri/core/device/serializers.py#L139 This would require adding a |
||
|
||
def create(self, validated_data): | ||
preset = validated_data.get("preset") | ||
name = validated_data.get("name") | ||
with transaction.atomic(): | ||
try: | ||
facility_dataset = FacilityDataset.objects.create(preset=preset) | ||
facility = Facility.objects.create(name=name, dataset=facility_dataset) | ||
facility.dataset.reset_to_default_settings(preset) | ||
except Exception as e: | ||
logger.error("Error occured while creating facility: %s", str(e)) | ||
raise ParseError("Error occured while creating facility") | ||
return facility | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Jaspreet-singh-1032 thanks for adding the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @akolson, I've made the changes you suggested by setting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By default the serializer will do introspection of the Django model, so that should be doing some validation to ensure it doesn't exceed the maximum length of the name attribute, and that it is a string, so I think Django REST Framework magic is probably doing the leg work for us here! |
||
|
||
class PublicFacilitySerializer(serializers.ModelSerializer): | ||
learner_can_login_with_no_password = serializers.SerializerMethodField() | ||
learner_can_sign_up = serializers.SerializerMethodField() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
<template> | ||
|
||
<KModal | ||
:title="$tr('createFacilityLabel')" | ||
:submitText="$tr('createFacilityButtonLabel')" | ||
:cancelText="coreString('closeAction')" | ||
size="medium" | ||
@submit="handleSubmit" | ||
@cancel="$emit('cancel')" | ||
> | ||
<p>{{ coreString('learningFacilityDescription') }}</p> | ||
<KTextbox | ||
ref="facilityNameTextBox" | ||
v-model="facilityName" | ||
:label="$tr('facilityNameFieldLabel')" | ||
:invalid="facilityNameInvalid" | ||
:invalidText="coreString('requiredFieldError')" | ||
:maxlength="50" | ||
/> | ||
<b>{{ $tr('learningEnvironmentHeader') }}</b> | ||
<KRadioButton | ||
v-model="preset" | ||
class="permission-preset-radio-button" | ||
:value="Presets.NONFORMAL" | ||
:label="$tr('nonFormalLabel')" | ||
:description="$tr('nonFormalDescription')" | ||
/> | ||
<KRadioButton | ||
v-model="preset" | ||
class="permission-preset-radio-button" | ||
:value="Presets.FORMAL" | ||
:label="$tr('formalLabel')" | ||
:description="$tr('formalDescription')" | ||
/> | ||
</KModal> | ||
|
||
</template> | ||
|
||
|
||
<script> | ||
|
||
import commonCoreStrings from 'kolibri.coreVue.mixins.commonCoreStrings'; | ||
import commonSyncElements from 'kolibri.coreVue.mixins.commonSyncElements'; | ||
import { Presets } from 'kolibri.coreVue.vuex.constants'; | ||
import { createFacility } from './api'; | ||
|
||
export default { | ||
name: 'CreateNewFacilityModal', | ||
mixins: [commonCoreStrings, commonSyncElements], | ||
data() { | ||
return { | ||
facilityName: '', | ||
preset: Presets.NONFORMAL, | ||
Presets, | ||
}; | ||
}, | ||
computed: { | ||
facilityNameInvalid() { | ||
return !this.facilityName || this.facilityName.trim() === ''; | ||
}, | ||
}, | ||
methods: { | ||
handleSubmit() { | ||
if (this.facilityNameInvalid) { | ||
return this.$refs.facilityNameTextBox.focus(); | ||
} | ||
const payload = { | ||
name: this.facilityName, | ||
preset: this.preset, | ||
}; | ||
createFacility(payload) | ||
.then(() => { | ||
this.$emit('success'); | ||
this.showSnackbarNotification('newLearningFacilityCreated'); | ||
}) | ||
.catch(error => { | ||
this.$store.dispatch('handleApiError', error); | ||
}); | ||
}, | ||
}, | ||
$trs: { | ||
facilityNameFieldLabel: { | ||
message: 'Learning facility name', | ||
context: 'The field where the admin adds the name of their facility.', | ||
}, | ||
learningEnvironmentHeader: { | ||
message: 'What kind of environment is your facility?', | ||
context: 'Title for facility environment.', | ||
}, | ||
formalLabel: { | ||
message: 'Formal', | ||
context: 'Label for the radio button option in the facility setup.', | ||
}, | ||
formalDescription: { | ||
message: 'Schools and other formal learning contexts.', | ||
context: "Option description text for 'Formal' facility types.", | ||
}, | ||
nonFormalLabel: { | ||
message: 'Non-formal', | ||
context: 'Label for the radio button option in the facility setup', | ||
}, | ||
nonFormalDescription: { | ||
message: | ||
'Libraries, orphanages, youth centers, computer labs, and other non-formal learning contexts.', | ||
|
||
context: "Option description text for 'Non-formal' facility types.", | ||
}, | ||
createFacilityLabel: { | ||
message: 'Create a new learning facility', | ||
context: 'Title for create facility modal', | ||
}, | ||
createFacilityButtonLabel: { | ||
message: 'create facility', | ||
context: 'Label for create facility submit button.', | ||
}, | ||
}, | ||
}; | ||
|
||
</script> | ||
|
||
|
||
<style lang="scss" scoped></style> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import client from 'kolibri.client'; | ||
import urls from 'kolibri.urls'; | ||
|
||
const url = urls['kolibri:core:facility-create-facility'](); | ||
|
||
export function createFacility(payload) { | ||
return client({ | ||
url, | ||
method: 'POST', | ||
data: payload, | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,11 +14,18 @@ | |
@click="showSyncAllModal = true" | ||
/> | ||
<KButton | ||
:text="getCommonSyncString('importFacilityAction')" | ||
hasDropdown | ||
:text="$tr('createFacilityLabel')" | ||
primary | ||
style="margin-top: 16px; margin-bottom: -16px;" | ||
@click="showImportModal = true" | ||
/> | ||
> | ||
<template #menu> | ||
<KDropdownMenu | ||
:options="options" | ||
@select="handleSelect" | ||
/> | ||
</template> | ||
</KButton> | ||
</KButtonGroup> | ||
</template> | ||
</HeaderWithOptions> | ||
|
@@ -144,6 +151,11 @@ | |
@success="handleStartImportSuccess" | ||
@cancel="showImportModal = false" | ||
/> | ||
<CreateNewFacilityModal | ||
v-if="showCreateFacilityModal" | ||
@success="handleCreateFacilitySuccess" | ||
@cancel="showCreateFacilityModal = false" | ||
/> | ||
|
||
<!-- NOTE similar code for KDP Registration in SyncInterface --> | ||
<template v-if="Boolean(facilityForRegister)"> | ||
|
@@ -195,13 +207,14 @@ | |
import { TaskStatuses, TaskTypes } from 'kolibri.utils.syncTaskUtils'; | ||
import some from 'lodash/some'; | ||
import DeviceAppBarPage from '../DeviceAppBarPage'; | ||
import { PageNames } from '../../constants'; | ||
import { PageNames, ImportFacility, CreateNewFacility } from '../../constants'; | ||
import { deviceString } from '../commonDeviceStrings'; | ||
import TasksBar from '../ManageContentPage/TasksBar'; | ||
import HeaderWithOptions from '../HeaderWithOptions'; | ||
import RemoveFacilityModal from './RemoveFacilityModal'; | ||
import SyncAllFacilitiesModal from './SyncAllFacilitiesModal'; | ||
import ImportFacilityModalGroup from './ImportFacilityModalGroup'; | ||
import CreateNewFacilityModal from './CreateNewFacilityModal'; | ||
import facilityTaskQueue from './facilityTasksQueue'; | ||
|
||
const Options = Object.freeze({ | ||
|
@@ -221,6 +234,7 @@ | |
DeviceAppBarPage, | ||
ConfirmationRegisterModal, | ||
CoreTable, | ||
CreateNewFacilityModal, | ||
HeaderWithOptions, | ||
FacilityNameAndSyncStatus, | ||
ImportFacilityModalGroup, | ||
|
@@ -235,6 +249,7 @@ | |
return { | ||
showSyncAllModal: false, | ||
showImportModal: false, | ||
showCreateFacilityModal: false, | ||
facilities: [], | ||
facilityForSync: null, | ||
facilityForRemoval: null, | ||
|
@@ -245,6 +260,18 @@ | |
}; | ||
}, | ||
computed: { | ||
options() { | ||
return [ | ||
{ | ||
label: this.$tr('importFacilityLabel'), | ||
value: ImportFacility, | ||
}, | ||
{ | ||
label: this.$tr('createNewFacilityLabel'), | ||
value: CreateNewFacility, | ||
}, | ||
]; | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would also be good to add the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the label values should be translated strings. |
||
pageTitle() { | ||
return deviceString('deviceManagementTitle'); | ||
}, | ||
|
@@ -344,6 +371,10 @@ | |
}); | ||
this.showImportModal = false; | ||
}, | ||
handleCreateFacilitySuccess() { | ||
this.showCreateFacilityModal = false; | ||
this.fetchFacilites(); | ||
}, | ||
manageSync(facilityId) { | ||
return { | ||
name: PageNames.MANAGE_SYNC_SCHEDULE, | ||
|
@@ -389,6 +420,13 @@ | |
this.$emit('failure'); | ||
}); | ||
}, | ||
handleSelect(option) { | ||
if (option.value == ImportFacility) { | ||
this.showImportModal = true; | ||
} else { | ||
this.showCreateFacilityModal = true; | ||
} | ||
}, | ||
}, | ||
$trs: { | ||
syncAllAction: { | ||
|
@@ -401,6 +439,18 @@ | |
context: | ||
"Notification that appears after a facility has been deleted. For example, \"Removed 'Zuk Village' from this device'.", | ||
}, | ||
createFacilityLabel: { | ||
message: 'ADD FACILITY', | ||
context: 'Label for a button used to create new facility.', | ||
}, | ||
importFacilityLabel: { | ||
message: 'Import facility', | ||
context: 'Label for the dropdown option of import facility', | ||
}, | ||
createNewFacilityLabel: { | ||
message: 'Create new facility', | ||
context: 'Label for the dropdown option of create new facility', | ||
}, | ||
}, | ||
}; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the only thing that this is missing is a permissions check here - we should require that
request.user.is_superuser
isTrue
otherwise, we should raise a permissions error before doing anything else here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent - yes using the permission classes in the decorator is a very neat way to do this!