Fix import element sorting when saving (for related items) #1078

MyPyDavid · 2024-07-24T10:45:31Z

Description

Fixes for the Import Feature

revert order function back to original (pre dev-2.2.0) append_element
that fixes the order of m2m parts in uploaded elements
needed to cast the order through field items to int
removes keys from the uploaded element that are not relevant for the import (like legacy keys)
type casting of certain fields (extra_fields) is done by the instance.full_clean() and these instance values are written back into the element[field]
- for example, to properly see the differences, the uploaded value needs to be case to float for FloatField otherwise a str will be compare to float

Motivation and Context

Made an overview of the models and the relations with the CLI tool graph_models

/manage.py graph_models conditions options domain questions tasks views -g --relation-fields-only=True --no-inheritance -x=created,updated,editors,sites,groups -l=dot -o rdmo_content_relations.svg

How has this been tested?

Screenshots (if appropriate)

Types of Changes

Bug fix (non-breaking change which fixes an issue)
Refactoring (no functional changes, no api changes)

Checklist

I have read the contributor guide.
My code follows the code style of this project.
I have added tests to cover my changes.
All new and existing tests passed.

Signed-off-by: David Wallace <[email protected]>

rdmo/core/imports.py

rdmo/core/constants.py

MyPyDavid · 2024-08-02T16:01:51Z

I broke the sorting with append_element with a typo before in the import feature branch.
Am now reverting it back to original sorting, but get a RecursionError for the legacy tests..

typo, it should be value instead of element_value in:

rdmo/rdmo/core/xml.py

Line 451 in ea118ed

if isinstance(element_value, dict):

…to legacy Signed-off-by: David Wallace <[email protected]>

Signed-off-by: David Wallace <[email protected]>

…fle to elements Signed-off-by: David Wallace <[email protected]>

Signed-off-by: David Wallace <[email protected]>

rdmo/management/tests/helpers_import_elements.py

rdmo/core/xml.py

rdmo/management/imports.py

jochenklar · 2024-08-07T09:13:23Z

rdmo/core/xml.py

@@ -257,6 +257,21 @@ def validate_pre_conversion_for_missing_key_in_legacy_elements(elements, version
            raise ValueError(f"Missing legacy elements, elements containing 'key' were expected for this XML with version {version} and elements {models_in_elements}.")   # noqa: E501


+def update_related_elements(elements: Dict, related_uri: str, related_model: str, related_key: str, field_name: str):


What ist this method used for?

I think there was something broken/missing with the current legacy xml parsing because the related elements are not written back into the ascending elements (following the current element Models).
For example, for the sections I added:

del element['catalog'] # sections do not have catalog anymore

This is correct right? They don't have catalog anymore..
But then the corresponding catalog needs to know about this section so I've added this function to "update" the catalog in that case.
The same then holds for pages, questionsets, questions etc.. right?

Still it's for the legacy xml's.. I could also revert if needed...

rename DEFAULT_XML_VERSION to LEGACY_XML_VERSION

rdmo/core/import_helpers.py

… cases Signed-off-by: David Wallace <[email protected]>

Signed-off-by: David Wallace <[email protected]>

package-lock.json

Signed-off-by: David Wallace <[email protected]>

rdmo/management/import_utils.py

…zation Signed-off-by: David Wallace <[email protected]>

MyPyDavid · 2024-08-14T14:15:12Z

rdmo/management/import_utils.py

+            try:
+                # Instantiate the DRF field and use it to validate and convert the value
+                drf_field = drf_field_class()
+                element[field_name] = drf_field.to_internal_value(element_field_value)


Ive added some deserialization via the DRF Fields in here, it happens only for the so-called 'extra fields' of a rdmo model.

I am arguing about creating to many functions all the time, but this thing could be a relatively generic function validate_with_serializer_field(instance, field_name) or something.

yes that's true, I can refactor it to that, as the last change of this PR

... as the last change of this PR

🙄

ok, Ive refactored it into a separate function, it could be moved to core it it's needed by other apps/functions..

jochenklar · 2024-08-15T08:44:39Z

rdmo/management/import_utils.py

+            try:
+                # Instantiate the DRF field and use it to validate and convert the value
+                drf_field = drf_field_class()
+                element[field_name] = drf_field.to_internal_value(element_field_value)


jochenklar · 2024-08-15T08:47:42Z

rdmo/management/import_utils.py

+            try:
+                # Instantiate the DRF field and use it to validate and convert the value
+                drf_field = drf_field_class()
+                element[field_name] = drf_field.to_internal_value(element_field_value)


I am arguing about creating to many functions all the time, but this thing could be a relatively generic function validate_with_serializer_field(instance, field_name) or something.

Signed-off-by: David Wallace <[email protected]>

MyPyDavid · 2024-08-15T14:38:11Z

thanks for the review!

fix(imports): fix ordering when saving

5fa19b6

Signed-off-by: David Wallace <[email protected]>

MyPyDavid added this to the RDMO 2.2.0 milestone Jul 24, 2024

MyPyDavid self-assigned this Jul 24, 2024

MyPyDavid added 4 commits July 24, 2024 18:46

fix(imports): cast order to int on target_element

af50cde

Signed-off-by: David Wallace <[email protected]>

style(imports): remove spaces around equals sign

3ed80b3

Signed-off-by: David Wallace <[email protected]>

fix(imports): remove unused keys from element

d752c4b

Signed-off-by: David Wallace <[email protected]>

fix(imports): add float casting for track changes

ea118ed

Signed-off-by: David Wallace <[email protected]>

MyPyDavid requested a review from jochenklar July 25, 2024 12:20

MyPyDavid marked this pull request as ready for review July 25, 2024 12:43

MyPyDavid commented Jul 25, 2024

View reviewed changes

rdmo/core/imports.py Outdated Show resolved Hide resolved

jochenklar reviewed Jul 26, 2024

View reviewed changes

rdmo/core/constants.py Show resolved Hide resolved

jochenklar reviewed Jul 26, 2024

View reviewed changes

rdmo/core/constants.py Show resolved Hide resolved

MyPyDavid added the status:work-in-progress label Jul 29, 2024

MyPyDavid added 5 commits August 5, 2024 10:52

refactor(core,xml): revert changes to ordering, add related elements …

d5ad915

…to legacy Signed-off-by: David Wallace <[email protected]>

refactor(imports): clean up funcs and args, change handling extra fields

3111074

Signed-off-by: David Wallace <[email protected]>

tests(import): add assertions for related items and add optional shuf…

bbd0f0a

…fle to elements Signed-off-by: David Wallace <[email protected]>

refactor(js,import): remove WarningsListGroup.defaultProps

718a252

Signed-off-by: David Wallace <[email protected]>

refactor(imports): compare by set in track_changes_m2m_instances

b8c38e8

Signed-off-by: David Wallace <[email protected]>

MyPyDavid requested a review from jochenklar August 6, 2024 13:05

jochenklar reviewed Aug 7, 2024

View reviewed changes

MyPyDavid added 3 commits August 8, 2024 17:43

tests(import,options): add assertions for related elements for create…

a2a3dcb

… cases Signed-off-by: David Wallace <[email protected]>

refactor(xml): rename and update legacy import

6c843b6

Signed-off-by: David Wallace <[email protected]>

tests(import,options): update assertions for create optionsets tests

16d96ef

Signed-off-by: David Wallace <[email protected]>

MyPyDavid commented Aug 13, 2024

View reviewed changes

package-lock.json Outdated Show resolved Hide resolved

MyPyDavid added 3 commits August 13, 2024 10:46

build(js): re-add package-lock.json

fa2b231

Signed-off-by: David Wallace <[email protected]>

tests(import,questions): add boolean shuffle parameter to create tests

3e4358d

Signed-off-by: David Wallace <[email protected]>

build(js): revert package-lock.json to dev-2.2.0

b6920df

Signed-off-by: David Wallace <[email protected]>

MyPyDavid commented Aug 13, 2024

View reviewed changes

rdmo/management/import_utils.py Outdated Show resolved Hide resolved

refactor(import,utils): use drf field class for extra field deseriali…

cd6a0a4

…zation Signed-off-by: David Wallace <[email protected]>

MyPyDavid added status:acceptance and removed status:work-in-progress labels Aug 14, 2024

MyPyDavid requested a review from jochenklar August 14, 2024 14:13

MyPyDavid commented Aug 14, 2024

View reviewed changes

jochenklar approved these changes Aug 15, 2024

View reviewed changes

refactor(import,utils): make dfr field validation into separate function

21f1f74

Signed-off-by: David Wallace <[email protected]>

MyPyDavid merged commit 2f71c09 into dev-2.2.0 Aug 15, 2024
18 checks passed

MyPyDavid deleted the fix_import_element_sorting branch August 22, 2024 07:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix import element sorting when saving (for related items) #1078

Fix import element sorting when saving (for related items) #1078

MyPyDavid commented Jul 24, 2024 •

edited

Loading

MyPyDavid commented Aug 2, 2024 •

edited

Loading

jochenklar Aug 7, 2024

MyPyDavid Aug 7, 2024 •

edited

Loading

MyPyDavid Aug 8, 2024 •

edited

Loading

MyPyDavid Aug 14, 2024

jochenklar Aug 15, 2024

jochenklar Aug 15, 2024

MyPyDavid Aug 15, 2024

jochenklar Aug 15, 2024

MyPyDavid Aug 15, 2024

jochenklar Aug 15, 2024

jochenklar Aug 15, 2024

MyPyDavid commented Aug 15, 2024

		@@ -257,6 +257,21 @@ def validate_pre_conversion_for_missing_key_in_legacy_elements(elements, version
		raise ValueError(f"Missing legacy elements, elements containing 'key' were expected for this XML with version {version} and elements {models_in_elements}.") # noqa: E501


		def update_related_elements(elements: Dict, related_uri: str, related_model: str, related_key: str, field_name: str):

Fix import element sorting when saving (for related items) #1078

Fix import element sorting when saving (for related items) #1078

Conversation

MyPyDavid commented Jul 24, 2024 • edited Loading

Description

Motivation and Context

How has this been tested?

Screenshots (if appropriate)

Types of Changes

Checklist

MyPyDavid commented Aug 2, 2024 • edited Loading

Choose a reason for hiding this comment

MyPyDavid Aug 7, 2024 • edited Loading

Choose a reason for hiding this comment

MyPyDavid Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MyPyDavid commented Aug 15, 2024

MyPyDavid commented Jul 24, 2024 •

edited

Loading

MyPyDavid commented Aug 2, 2024 •

edited

Loading

MyPyDavid Aug 7, 2024 •

edited

Loading

MyPyDavid Aug 8, 2024 •

edited

Loading