-
Notifications
You must be signed in to change notification settings - Fork 0
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
Scheme Label Editor & Reference Datatype Widget #169
Changes from 35 commits
51ca818
0a6e33b
b98ed49
c2093dd
6c10a1f
9ff15fa
310401c
6b74f41
abf06f7
e3c119d
5d8c7c3
60536ab
2a2b889
20227b0
faae95c
1f6b076
4ea7d6c
f067d49
be6d74e
0500832
eb23d27
57348b8
f901175
c290ff6
d325db5
a3c7cb4
86f230e
c07c6ae
510f59c
65f5e83
e41c50f
8ffb6c1
557a2a0
f71bacb
8418e8a
96dd5b2
02a721e
600fe2f
46e44f1
bff6057
57ea8cc
35a1237
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
from rest_framework.exceptions import ValidationError | ||
|
||
from arches_references.models import ListItem | ||
|
||
from arches.app.models.models import ResourceInstance, TileModel | ||
from arches.app.models.serializers import ArchesModelSerializer, ArchesTileSerializer | ||
|
||
|
@@ -49,6 +53,35 @@ class Meta: | |
root_node = "appellative_status" | ||
fields = "__all__" | ||
|
||
def validate(self, data): | ||
data = super().validate(data) | ||
graph_slug = self.Meta.graph_slug | ||
PREF_LABEL_LIST_ITEM = ListItem.objects.get(list_item_values__value="prefLabel") | ||
|
||
if data: | ||
new_label_language = data["appellative_status_ascribed_name_language"][0] | ||
new_label_type = data["appellative_status_ascribed_relation"][0] | ||
|
||
resource_instance_id = self.instance.resourceinstance.pk | ||
resource_instance = self.instance.resourceinstance.__class__.as_model( | ||
graph_slug, resource_ids=[resource_instance_id] | ||
).get(pk=resource_instance_id) | ||
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'm looking into adding a developer convenience for this, either a method 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. (Not suggesting to block this PR on it.) 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. @jacobtylerwalls I swear this was working yesterday, but when I try to create a new label for an existing scheme self.instance is 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. Done! I think you and I were only testing updates, not creates. I also added a little developer convenience for now. |
||
current_labels = resource_instance.appellative_status | ||
|
||
for label in current_labels: | ||
label_language = label.appellative_status_ascribed_name_language[0] | ||
label_type = label.appellative_status_ascribed_relation[0] | ||
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. These fail with KeyError if not provided. If these fields are truly required at the node config level (seems like a model issue that they aren't) then the arches queryset layer that handles the tile validation logic will reject it. But since that layer is later, this layer needs to be a little more forgiving. Unless! We "override" the fields in this serializer to enforce that they're required at the outermost layer. Something like: diff --git a/arches_lingo/serializers.py b/arches_lingo/serializers.py
index 1017492..db61b2b 100644
--- a/arches_lingo/serializers.py
+++ b/arches_lingo/serializers.py
@@ -1,3 +1,4 @@
+from rest_framework import serializers
from rest_framework.exceptions import ValidationError
from arches_references.models import ListItem
@@ -47,6 +48,10 @@ class SchemeLabelSerializer(ArchesModelSerializer):
class SchemeLabelTileSerializer(ArchesTileSerializer):
+ # Get this from a mapping from datatypes to serializer fields
+ # then add required=True to fail faster
+ appellative_status_ascribed_name_language = serializers.JSONField(allow_null=True, required=True)
+
class Meta:
model = TileModel
graph_slug = "scheme" If we do that, then your business logic doesn't need to guard against missing values, because we fail before getting here (Looks like we need to improve the error toast to actually return the error message from the server keyed on field name instead of just "Bad Request".) Nice to have some options here. I'm not convinced overriding the serializer field is better; it's probably easier to just fix your 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. Added a fix for this, but we probably want a dataclass or something to prevent this awkward double fallback. |
||
if ( | ||
data["tileid"] != label.tileid | ||
and new_label_type["uri"] == PREF_LABEL_LIST_ITEM.uri | ||
and label_type["uri"] == PREF_LABEL_LIST_ITEM.uri | ||
and label_language["uri"] == new_label_language["uri"] | ||
): | ||
raise ValidationError( | ||
"A preferred label with the same language already exists for this scheme." | ||
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 localized this and borrowed the wording from arches-references to prevent creating a second translatable string. |
||
) | ||
return data | ||
|
||
|
||
class SchemeNoteSerializer(ArchesModelSerializer): | ||
class Meta: | ||
|
@@ -88,3 +121,19 @@ class Meta: | |
graph_slug = "concept" | ||
nodegroups = "__all__" | ||
fields = "__all__" | ||
|
||
|
||
class PersonRdmSystemSerializer(ArchesModelSerializer): | ||
class Meta: | ||
model = ResourceInstance | ||
graph_slug = "person" | ||
nodegroups = "__all__" | ||
fields = "__all__" | ||
|
||
|
||
class GroupRdmSystemSerializer(ArchesModelSerializer): | ||
class Meta: | ||
model = ResourceInstance | ||
graph_slug = "group" | ||
nodegroups = "__all__" | ||
fields = "__all__" |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
<script setup lang="ts"> | ||
import DateDatatypeViewer from "@/arches_lingo/components/generic/date-datatype/DateDatatypeViewer.vue"; | ||
import DateDatatypeEditor from "@/arches_lingo/components/generic/date-datatype/DateDatatypeEditor.vue"; | ||
|
||
import { EDIT, VIEW } from "@/arches_lingo/constants.ts"; | ||
|
||
import type { DataComponentMode } from "@/arches_lingo/types.ts"; | ||
|
||
interface Props { | ||
dateFormat?: string; | ||
mode?: DataComponentMode; | ||
value?: string; | ||
ptAriaLabeledBy?: string; | ||
} | ||
|
||
withDefaults(defineProps<Props>(), { | ||
dateFormat: "yy-mm-dd", | ||
mode: VIEW, | ||
value: "", | ||
}); | ||
|
||
const emits = defineEmits(["update"]); | ||
|
||
const onUpdate = (val: string) => { | ||
emits("update", val); | ||
}; | ||
</script> | ||
|
||
<template> | ||
<div> | ||
<div v-if="mode === VIEW"> | ||
<DateDatatypeViewer | ||
:date-format="dateFormat" | ||
:value="value" | ||
/> | ||
</div> | ||
<div v-if="mode === EDIT"> | ||
<DateDatatypeEditor | ||
:date-format="dateFormat" | ||
:value="value" | ||
:pt-aria-labeled-by="ptAriaLabeledBy" | ||
@update="onUpdate" | ||
/> | ||
</div> | ||
</div> | ||
</template> |
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.
Need to catch MultipleObjectsReturned or otherwise define which list item wins.
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.
Pushed an update for this, a bit silly, but there are probably better things we can do in the future around identifying a list with a natural key (should name be unique, assuming it's not localized?)