You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airavata.apache.org by ma...@apache.org on 2022/06/07 21:17:52 UTC
[airavata-django-portal] 03/03: AIRAVATA-3566 Refactor code, pushing logic into model
This is an automated email from the ASF dual-hosted git repository.
machristie pushed a commit to branch AIRAVATA-3562
in repository https://gitbox.apache.org/repos/asf/airavata-django-portal.git
commit 09582cf39fef01ac96dcd4d3c300ce78bd01d796
Author: Marcus Christie <ma...@apache.org>
AuthorDate: Tue Jun 7 17:17:36 2022 -0400
AIRAVATA-3566 Refactor code, pushing logic into model
---
.../components/users/ExtendedUserProfilePanel.vue | 69 +-----
.../js/models/ExtendedUserProfileValue.js | 2 +-
django_airavata/apps/auth/models.py | 57 +++++
django_airavata/apps/auth/serializers.py | 23 +-
django_airavata/apps/auth/tests/test_models.py | 240 +++++++++++++++++++++
5 files changed, 311 insertions(+), 80 deletions(-)
diff --git a/django_airavata/apps/admin/static/django_airavata_admin/src/components/users/ExtendedUserProfilePanel.vue b/django_airavata/apps/admin/static/django_airavata_admin/src/components/users/ExtendedUserProfilePanel.vue
index 4b4b52a6..c3e376c1 100644
--- a/django_airavata/apps/admin/static/django_airavata_admin/src/components/users/ExtendedUserProfilePanel.vue
+++ b/django_airavata/apps/admin/static/django_airavata_admin/src/components/users/ExtendedUserProfilePanel.vue
@@ -2,21 +2,17 @@
<b-card header="Extended User Profile">
<b-table :items="items" :fields="fields" small borderless>
<template #cell(value)="{ value, item }">
- <i v-if="item.valid" class="fas fa-check text-success"></i>
+ <!-- only show a valid checkmark when there is a user provided value -->
+ <i v-if="value && item.valid" class="fas fa-check text-success"></i>
<i v-if="!item.valid" class="fas fa-times text-danger"></i>
- <template v-if="item.type === 'text' || item.type === 'single_choice'">
- {{ value }}
- </template>
- <template v-else-if="item.type === 'user_agreement'">
- <b-checkbox class="ml-2 d-inline" :checked="value" disabled />
- </template>
- <template v-else-if="item.type === 'multi_choice'">
+ <template v-if="Array.isArray(value)">
<ul>
- <li v-for="result in item.value" :key="result">
+ <li v-for="result in value" :key="result">
{{ result }}
</li>
</ul>
</template>
+ <template v-else> {{ value }} </template>
</template>
</b-table>
</b-card>
@@ -50,12 +46,12 @@ export default {
if (this.extendedUserProfileFields && this.extendedUserProfileValues) {
const items = [];
for (const field of this.extendedUserProfileFields) {
- const { value, valid } = this.getValue({ field });
+ const value = this.getValue(field);
items.push({
name: field.name,
- value,
- valid,
- type: field.field_type,
+ value: value ? value.value_display : null,
+ // if no value, consider it invalid only if it is required
+ valid: value ? value.valid : !field.required,
});
}
return items;
@@ -69,53 +65,10 @@ export default {
"loadExtendedUserProfileFields",
"loadExtendedUserProfileValues",
]),
- getValue({ field }) {
- if (!this.extendedUserProfileValues) {
- return null;
- }
- const value = this.extendedUserProfileValues.find(
+ getValue(field) {
+ return this.extendedUserProfileValues.find(
(v) => v.ext_user_profile_field === field.id
);
- if (value && value.value_type === "text") {
- return { value: value.text_value, valid: value.valid };
- }
- if (value && value.value_type === "user_agreement") {
- return { value: value.user_agreement, valid: value.valid };
- }
- if (value && value.value_type === "single_choice") {
- if (value.other_value) {
- return { value: `Other: ${value.other_value}`, valid: value.valid };
- } else if (value.choices && value.choices.length === 1) {
- const displayText = this.getChoiceDisplayText({
- field,
- choiceId: value.choices[0],
- });
- if (displayText) {
- return { value: displayText, valid: value.valid };
- }
- }
- return { value: null, valid: value.valid };
- }
- if (value && value.value_type === "multi_choice") {
- const results = [];
- if (value.choices) {
- for (const choiceId of value.choices) {
- const displayText = this.getChoiceDisplayText({ field, choiceId });
- if (displayText) {
- results.push(displayText);
- }
- }
- }
- if (value.other_value) {
- results.push(`Other: ${value.other_value}`);
- }
- return { value: results, valid: value.valid };
- }
- return { value: null, valid: !field.required };
- },
- getChoiceDisplayText({ field, choiceId }) {
- const choice = field.choices.find((c) => c.id === choiceId);
- return choice ? choice.display_text : null;
},
},
};
diff --git a/django_airavata/apps/api/static/django_airavata_api/js/models/ExtendedUserProfileValue.js b/django_airavata/apps/api/static/django_airavata_api/js/models/ExtendedUserProfileValue.js
index 2a1d56c9..bb417a6d 100644
--- a/django_airavata/apps/api/static/django_airavata_api/js/models/ExtendedUserProfileValue.js
+++ b/django_airavata/apps/api/static/django_airavata_api/js/models/ExtendedUserProfileValue.js
@@ -1,6 +1,5 @@
import BaseModel from "./BaseModel";
-// TODO: do we need this?
const FIELDS = [
"id",
"value_type",
@@ -10,6 +9,7 @@ const FIELDS = [
"other_value",
"agreement_value",
"valid",
+ "value_display",
];
export default class ExtendedUserProfileValue extends BaseModel {
diff --git a/django_airavata/apps/auth/models.py b/django_airavata/apps/auth/models.py
index 8d7cbb41..3bae9e25 100644
--- a/django_airavata/apps/auth/models.py
+++ b/django_airavata/apps/auth/models.py
@@ -265,6 +265,63 @@ class ExtendedUserProfileValue(models.Model):
else:
raise Exception("Could not determine value_type")
+ @property
+ def value_display(self):
+ if self.value_type == 'text':
+ return self.text.text_value
+ elif self.value_type == 'single_choice':
+ if self.single_choice.choice:
+ try:
+ choice = self.ext_user_profile_field.single_choice.choices.get(id=self.single_choice.choice)
+ return choice.display_text
+ except ExtendedUserProfileSingleChoiceFieldChoice.DoesNotExist:
+ return None
+ elif self.single_choice.other_value:
+ return f"Other: {self.single_choice.other_value}"
+ elif self.value_type == 'multi_choice':
+ result = []
+ if self.multi_choice.choices:
+ mc_field = self.ext_user_profile_field.multi_choice
+ for choice_value in self.multi_choice.choices.all():
+ try:
+ choice = mc_field.choices.get(id=choice_value.value)
+ result.append(choice.display_text)
+ except ExtendedUserProfileMultiChoiceFieldChoice.DoesNotExist:
+ continue
+ if self.multi_choice.other_value:
+ result.append(f"Other: {self.multi_choice.other_value}")
+ return result
+ elif self.value_type == 'user_agreement':
+ if self.user_agreement.agreement_value:
+ return "Yes"
+ else:
+ return "No"
+ return None
+
+ @property
+ def valid(self):
+ if self.ext_user_profile_field.required:
+ if self.value_type == 'text':
+ return self.text.text_value and len(self.text.text_value.strip()) > 0
+ if self.value_type == 'single_choice':
+ choice_exists = (self.single_choice.choice and
+ self.ext_user_profile_field.single_choice.choices
+ .filter(id=self.single_choice.choice).exists())
+ has_other = (self.ext_user_profile_field.single_choice.other and
+ self.single_choice.other_value and
+ len(self.single_choice.other_value.strip()) > 0)
+ return choice_exists or has_other
+ if self.value_type == 'multi_choice':
+ choice_ids = list(map(lambda c: c.value, self.multi_choice.choices.all()))
+ choice_exists = self.ext_user_profile_field.multi_choice.choices.filter(id__in=choice_ids).exists()
+ has_other = (self.ext_user_profile_field.multi_choice.other and
+ self.multi_choice.other_value and
+ len(self.multi_choice.other_value.strip()) > 0)
+ return choice_exists or has_other
+ if self.value_type == 'user_agreement':
+ return self.user_agreement.agreement_value is True
+ return True
+
class ExtendedUserProfileTextValue(ExtendedUserProfileValue):
value_ptr = models.OneToOneField(ExtendedUserProfileValue,
diff --git a/django_airavata/apps/auth/serializers.py b/django_airavata/apps/auth/serializers.py
index fe538aa7..dbafa415 100644
--- a/django_airavata/apps/auth/serializers.py
+++ b/django_airavata/apps/auth/serializers.py
@@ -230,13 +230,12 @@ class ExtendedUserProfileValueSerializer(serializers.ModelSerializer):
choices = serializers.ListField(child=serializers.IntegerField(), required=False)
other_value = serializers.CharField(required=False, allow_blank=True)
agreement_value = serializers.BooleanField(required=False)
- valid = serializers.SerializerMethodField()
class Meta:
model = models.ExtendedUserProfileValue
fields = ['id', 'value_type', 'ext_user_profile_field', 'text_value',
- 'choices', 'other_value', 'agreement_value', 'valid']
- read_only_fields = ['value_type']
+ 'choices', 'other_value', 'agreement_value', 'valid', 'value_display']
+ read_only_fields = ['value_type', 'value_display']
def to_representation(self, instance):
result = super().to_representation(instance)
@@ -353,21 +352,3 @@ class ExtendedUserProfileValueSerializer(serializers.ModelSerializer):
if not ext_user_profile_field.multi_choice.choices.filter(id=choice, deleted=False).exists():
raise serializers.ValidationError({'choices': 'Invalid choice.'})
return attrs
-
- # TODO: add label and display_value to serializer?
- def get_valid(self, value: models.ExtendedUserProfileValue):
- # TODO: move these to the model
- if value.ext_user_profile_field.required:
- if value.ext_user_profile_field.field_type == 'text':
- return value.text.text_value and len(value.text.text_value.strip()) > 0
- if value.ext_user_profile_field.field_type == 'single_choice':
- return value.single_choice.choice is not None or (
- value.single_choice.other_value and
- len(value.single_choice.other_value.strip()) > 0)
- if value.ext_user_profile_field.field_type == 'multi_choice':
- return len(value.multi_choice.choices) > 0 or (
- value.multi_choice.other_value and
- len(value.multi_choice.other_value.strip()) > 0)
- if value.ext_user_profile_field.field_type == 'user_agreement':
- return value.user_agreement.agreement_value is True
- return True
diff --git a/django_airavata/apps/auth/tests/test_models.py b/django_airavata/apps/auth/tests/test_models.py
new file mode 100644
index 00000000..73b18331
--- /dev/null
+++ b/django_airavata/apps/auth/tests/test_models.py
@@ -0,0 +1,240 @@
+from django.contrib.auth import get_user_model
+from django.test import TestCase
+
+from django_airavata.apps.auth import models
+
+
+class ExtendedUserProfileValueTestCase(TestCase):
+
+ def setUp(self) -> None:
+ User = get_user_model()
+ user = User.objects.create_user("testuser")
+ self.user_profile = models.UserProfile.objects.create(user=user)
+
+ def test_value_display_of_text_value(self):
+ field = models.ExtendedUserProfileTextField.objects.create(
+ name="test", order=1)
+ value = models.ExtendedUserProfileTextValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ text_value="Some random answer.")
+ self.assertEqual(value.value_display, value.text_value)
+
+ def test_value_display_of_single_choice_with_choice(self):
+ field = models.ExtendedUserProfileSingleChoiceField.objects.create(
+ name="test", order=1)
+ field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ choice_two = field.choices.get(display_text="Choice #2")
+ value = models.ExtendedUserProfileSingleChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ choice=choice_two.id)
+ self.assertEqual(value.value_display, choice_two.display_text)
+
+ def test_value_display_of_single_choice_with_non_existent_choice(self):
+ field = models.ExtendedUserProfileSingleChoiceField.objects.create(
+ name="test", order=1)
+ field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileSingleChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ choice=-1)
+ self.assertEqual(value.value_display, None)
+
+ def test_value_display_of_single_choice_with_other(self):
+ field = models.ExtendedUserProfileSingleChoiceField.objects.create(
+ name="test", order=1)
+ field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileSingleChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ other_value="Write-in value")
+ self.assertEqual(value.value_display, "Other: Write-in value")
+
+ def test_value_display_of_multi_choice_with_choices(self):
+ field = models.ExtendedUserProfileMultiChoiceField.objects.create(
+ name="test", order=1)
+ choice_one = field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ choice_three = field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileMultiChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile)
+ value.choices.create(value=choice_one.id)
+ value.choices.create(value=choice_three.id)
+ self.assertListEqual(value.value_display, [choice_one.display_text, choice_three.display_text])
+
+ def test_value_display_of_multi_choice_with_other(self):
+ field = models.ExtendedUserProfileMultiChoiceField.objects.create(
+ name="test", order=1)
+ field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileMultiChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ other_value="Some write-in value.")
+ self.assertListEqual(value.value_display, ["Other: Some write-in value."])
+
+ def test_value_display_of_multi_choice_with_choices_and_other(self):
+ field = models.ExtendedUserProfileMultiChoiceField.objects.create(
+ name="test", order=1)
+ choice_one = field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ choice_three = field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileMultiChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ other_value="Some write-in value.")
+ value.choices.create(value=choice_one.id)
+ value.choices.create(value=choice_three.id)
+ self.assertListEqual(value.value_display, [choice_one.display_text, choice_three.display_text, "Other: Some write-in value."])
+
+ def test_value_display_of_multi_choice_with_non_existent_choices(self):
+ field = models.ExtendedUserProfileMultiChoiceField.objects.create(
+ name="test", order=1)
+ choice_one = field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ choice_three = field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileMultiChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile)
+ value.choices.create(value=choice_one.id)
+ value.choices.create(value=choice_three.id)
+ value.choices.create(value=-1)
+ self.assertListEqual(value.value_display, [choice_one.display_text, choice_three.display_text])
+
+ def test_value_display_of_user_agreement_with_no(self):
+ field = models.ExtendedUserProfileAgreementField.objects.create(
+ name="test", order=1)
+ value = models.ExtendedUserProfileAgreementValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ agreement_value=False)
+ self.assertEqual(value.value_display, "No")
+
+ def test_value_display_of_user_agreement_with_yes(self):
+ field = models.ExtendedUserProfileAgreementField.objects.create(
+ name="test", order=1)
+ value = models.ExtendedUserProfileAgreementValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ agreement_value=True)
+ self.assertEqual(value.value_display, "Yes")
+
+ def test_valid_of_text(self):
+ field = models.ExtendedUserProfileTextField.objects.create(
+ name="test", order=1, required=True)
+ value = models.ExtendedUserProfileTextValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ text_value="Some value")
+ self.assertTrue(value.valid)
+
+ def test_valid_of_text_empty(self):
+ field = models.ExtendedUserProfileTextField.objects.create(
+ name="test", order=1, required=True)
+ value = models.ExtendedUserProfileTextValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ text_value="")
+ self.assertFalse(value.valid)
+
+ def test_valid_of_text_empty_no_required(self):
+ field = models.ExtendedUserProfileTextField.objects.create(
+ name="test", order=1, required=False)
+ value = models.ExtendedUserProfileTextValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ text_value="")
+ self.assertTrue(value.valid)
+
+ def test_valid_of_single_choice_none(self):
+ field = models.ExtendedUserProfileSingleChoiceField.objects.create(
+ name="test", order=1, required=True)
+ value = models.ExtendedUserProfileSingleChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile)
+ self.assertFalse(value.valid)
+
+ def test_valid_of_single_choice_with_choice(self):
+ field = models.ExtendedUserProfileSingleChoiceField.objects.create(
+ name="test", order=1, required=True)
+ field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ choice_two = field.choices.get(display_text="Choice #2")
+ value = models.ExtendedUserProfileSingleChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ choice=choice_two.id)
+ self.assertTrue(value.valid)
+
+ def test_valid_of_single_choice_with_non_existent_choice(self):
+ field = models.ExtendedUserProfileSingleChoiceField.objects.create(
+ name="test", order=1, required=True)
+ field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileSingleChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ choice=-1)
+ self.assertFalse(value.valid)
+
+ def test_valid_of_single_choice_with_other(self):
+ field = models.ExtendedUserProfileSingleChoiceField.objects.create(
+ name="test", order=1, required=True, other=True)
+ field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileSingleChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ other_value="Some write-in value.")
+ self.assertTrue(value.valid)
+
+ def test_valid_of_single_choice_with_other_but_not_allowed(self):
+ # Configure field so that 'Other' isn't an option
+ field = models.ExtendedUserProfileSingleChoiceField.objects.create(
+ name="test", order=1, required=True, other=False)
+ field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileSingleChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ other_value="Some write-in value.")
+ self.assertFalse(value.valid)
+
+ def test_valid_of_multi_choice_with_none(self):
+ field = models.ExtendedUserProfileMultiChoiceField.objects.create(
+ name="test", order=1, required=True)
+ field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileMultiChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile)
+ self.assertFalse(value.valid)
+
+ def test_valid_of_multi_choice_with_some(self):
+ field = models.ExtendedUserProfileMultiChoiceField.objects.create(
+ name="test", order=1, required=True)
+ choice_one = field.choices.create(display_text="Choice #1", order=1)
+ choice_two = field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileMultiChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile)
+ value.choices.create(value=choice_one.id)
+ value.choices.create(value=choice_two.id)
+ self.assertTrue(value.valid)
+
+ def test_valid_of_multi_choice_with_non_existent_choice(self):
+ field = models.ExtendedUserProfileMultiChoiceField.objects.create(
+ name="test", order=1, required=True)
+ field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileMultiChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile)
+ value.choices.create(value=-1)
+ self.assertFalse(value.valid)
+
+ def test_valid_of_multi_choice_with_other(self):
+ field = models.ExtendedUserProfileMultiChoiceField.objects.create(
+ name="test", order=1, required=True, other=True)
+ field.choices.create(display_text="Choice #1", order=1)
+ field.choices.create(display_text="Choice #2", order=2)
+ field.choices.create(display_text="Choice #3", order=3)
+ value = models.ExtendedUserProfileMultiChoiceValue.objects.create(
+ ext_user_profile_field=field, user_profile=self.user_profile,
+ other_value="Some write-in value.")
+ self.assertTrue(value.valid)