Move related items from one attribute to another via CLI command (merge_attributes) #991

MyPyDavid · 2024-05-14T13:55:02Z

Description

Related issue: #990

Motivation and Context

How has this been tested?

Screenshots (if appropriate)

CLI shell command:

python ../muster-mandant/manage.py merge_attributes --source https://foo-merge.com/terms/domain/blocks/c5 --target https://foo-merge.com/terms/domain/blocks/c4 -v2 --delete --view

Part of the help message:

usage: manage.py merge_attributes [-h] --source SOURCE --target TARGET [--save] [--delete] [--view] [--version] [-v {0,1,2,3}] [--settings SETTINGS] [--pythonpath PYTHONPATH] [--traceback] [--no-color] [--force-color]
                                  [--skip-checks]

Replace an attribute with another attribute across multiple models

options:
  -h, --help            show this help message and exit
  --source SOURCE       The URI of the source attribute that will be replaced by the target and will be deleted
  --target TARGET       The URI of the target attribute that will be used to replace the source
  --save                If specified, the changes will be saved. If not specified, the command will not make any changes in the database.
  --delete              If specified, the source attribute will be deleted. If not specified, the command will not delete the source attribute.
  --view                If specified, the changes will be applied to the affected Views as well.
  --version             Show program's version number and exit.
  -v {0,1,2,3}, --verbosity {0,1,2,3}

....

Command:

python ../muster-mandant/manage.py merge_attributes --source https://foo-merge.com/terms/domain/blocks/c5 --target https://foo-merge.com/terms/domain/blocks/c4 -v2 --save --delete --view

logs:

[2024-06-06 15:31:11,702] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Question(id=1) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,711] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Question(id=18) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,724] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=1) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,733] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=116) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,740] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=242) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,747] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=243) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,754] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=244) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,763] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=245) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,769] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=246) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,776] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=247) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,783] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=248) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,790] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=249) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,798] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=359) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,805] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=456) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,813] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 on Value(id=465) was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,821] INFO: Attribute https://foo-merge.com/terms/domain/blocks/c5 in View(id=1) template was replaced with https://foo-merge.com/terms/domain/blocks/c4.
[2024-06-06 15:31:11,839] INFO: Source attribute https://foo-merge.com/terms/domain/blocks/c5 was deleted.

printed messages:

Successfully replaced source attribute https://foo-merge.com/terms/domain/blocks/c5 with https://foo-merge.com/terms/domain/blocks/c4.
Source attribute https://foo-merge.com/terms/domain/blocks/c5 was deleted.
Affected elements: Question(2), Value(13), View(1), Project(7)
Affected Projects:
        1       Test
        2       Parent
        3       Child1
        4       Child2
        5       Child11
        12      test
        14      test foo
Affected Views:
        1       http://example.com/terms/views/view_a

Merge attribute results for model Question (2)

Question(id=1)
instance=http://example.com/terms/questions/catalog/individual/text/text
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True


Question(id=18)
instance=http://example.com/terms/questions/catalog/set/individual-single/text
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True


Merge attribute results for model Value (13)

Value(id=1)
instance=Test / - /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=1) Test

Value(id=116)
instance=Test / Test / A first snapshot /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=1) Test

Value(id=242)
instance=Parent / - /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=2) Parent

Value(id=243)
instance=Parent / Parent / Test /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=2) Parent

Value(id=244)
instance=Child1 / Child1 / Test /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=3) Child1

Value(id=245)
instance=Child2 / Child2 / Test /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=4) Child2

Value(id=246)
instance=Child11 / Child11 / Test /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=5) Child11

Value(id=247)
instance=Child1 / - /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=3) Child1

Value(id=248)
instance=Child2 / - /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=4) Child2

Value(id=249)
instance=Child11 / - /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=5) Child11

Value(id=359)
instance=Test / Test / A second snapshot /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=1) Test

Value(id=456)
instance=test / - /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=12) test

Value(id=465)
instance=test foo / - /  / 0 / 0
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True
Project(id=14) test foo

Merge attribute results for model View (1)

View(id=1)
instance=http://example.com/terms/views/view_a
source=https://foo-merge.com/terms/domain/blocks/c5
target=https://foo-merge.com/terms/domain/blocks/c4
saved=True

Types of Changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Code style update (formatting, renaming)
Refactoring (no functional changes, no api changes)
Build related changes
Documentation content changes
Other (please describe):

Checklist

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

MyPyDavid · 2024-05-15T09:56:09Z

this has low-prio if it's not relevant for any other RDMO Instance..
I can copy this copy this code in the rdmo-app and run the command from there for our maintenance/cleaning purposes

jochenklar · 2024-05-15T10:30:50Z

I will have a look. Btw: locally you can also use the runscript function of django-extensions which is pretty neat.

rdmo/management/management/commands/merge_attributes.py

jochenklar · 2024-05-17T15:19:25Z

rdmo/management/management/commands/merge_attributes.py

+    return replace_attribute_on_elements(source, target, REPLACE_ATTRIBUTE_MODEL_HELPERS, save)
+
+
+class Command(BaseCommand):


This should be at the top of the file.

jochenklar · 2024-05-17T15:20:34Z

rdmo/management/management/commands/merge_attributes.py

+                        f"{affect_elements_msg}\n"
+                        f"{affected_projects_msg}"
+                    ))
+        except ValueError as e:


Is this meant to catch all ValueErrors in the whole code? Exceptions should be more specific to functions or a few lines of code.

no 😅 , thought add a custom exception at some point, but now I'm raising only CommandErrors in the handle method

rdmo/management/management/commands/merge_attributes.py

jochenklar · 2024-05-17T15:22:33Z

rdmo/management/management/commands/merge_attributes.py

+    return msg
+
+
+def replace_attribute_on_elements(source, target, model_helpers_with_attributes, save):


Also not needed, the containing code should be just in handle.

yes, moved most of the code to handle

jochenklar · 2024-05-17T15:28:34Z

rdmo/management/management/commands/merge_attributes.py

+
+ModelHelper = namedtuple('ModelHelper', ['model', 'lookup_field', 'field_introspection', 'replacement_function'])
+
+REPLACE_ATTRIBUTE_MODEL_HELPERS = [


This dict can be replaced with one:

if is_a_view: ... else: ...

in then main loop.

I've replaced these helpers with

related_model_fields = [i for i in Attribute._meta.get_fields() \ if i.is_relation and not i.many_to_many and i.related_model is not Attribute]

and handle the View separate and explicit.

seems agreeable to you?

jochenklar · 2024-05-17T15:29:22Z

rdmo/management/management/commands/merge_attributes.py

+    return instance
+
+
+def replace_attribute_on_element_instances(model,


This function is also not needed, it is called only one time.

yes, refactored it .. there is still a function called one time, replace_attribute_in_view_template

yes, but now that you can read it from top to bottom, it is much better readable.

rdmo/management/management/commands/merge_attributes.py

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

jochenklar · 2024-05-30T08:56:33Z

rdmo/management/management/commands/merge_attributes.py

+
+        if save and all(a['saved'] for i in results.values() for a in i):
+            try:
+                source.delete()


Just a thought: if the attribute is deleted afterwards, it could have been moved to a different place in the tree without touching the foreign_keys, right? Then only the rewriting of the templates would be needed.

I'm not sure what you mean by "it could have been moved to a different place in the tree without touching the foreign_keys" , maybe I'm missing some knowledge about logic of the tree and foreign_keys here but I think it's the essential part of this feature..

The function replace_attribute_on_related_model_instances could maybe better be called assign_related_model_instances_to_attribute (to the target) . Or it should, in addition, set the related fields on the source to None (which seems redundant before source.delete()).

You can move Attributes in the RDMO management. This changes their URI, but all the relations are kept because the use the id field in the database.

ah like that, but that is just a rename of the URI then right?

However, in this use-case I have an existing URI foo (that has related elements already) and bar which is sort of a duplicate with its own related elements. When I want to delete bar and assign all its related elements to foo, I can't just rename bar to foo in this case..

Ah ok, you want to merge the old URI to into the new one that already has elements! Now everything makes sense. I would make a --delete switch or so. Makes more sense to me.

I would just use --delete this is pretty common in cli tools.

I would add --delete and set --dry-run as default to true as args

but if --dry-run is default and a store_true you could not unset it.

by using store_false maybe?

by GPT:

from django.core.management.base import BaseCommand class Command(BaseCommand): help = 'A custom command that uses a store_false argument with a default of True' def add_arguments(self, parser): # Adding a `store_false` argument that defaults to True parser.add_argument( '--no-feature', action='store_false', dest='feature_enabled', default=True, help='Disable the feature (default: feature is enabled)' ) def handle(self, *args, **options): feature_enabled = options['feature_enabled'] if feature_enabled: self.stdout.write(self.style.SUCCESS('Feature is enabled')) else: self.stdout.write(self.style.WARNING('Feature is disabled'))

hm, then it would be --no-dry-run ..
I have now --save , but I'll change that to --delete that will save the updated instances and delete the source attribute afterwards.

MyPyDavid · 2024-06-06T09:27:06Z

We have tried this out yesterday and have some changes to the feature request.

add logging also to a file with the id`s of affected elements, with a before and after comparison.
the View should be left out of this script because selecting and changing the path of an URI in the View templates can be ambiguous. The URI prefix is somehow missing in the Views. The output could include e.g. "Views discovered that use path of source attribute ..."

jochenklar · 2024-06-06T09:48:00Z

The uri prefix is not needed in the view. In fact, we added this later. So I guess many views will only use the path, but actually this could be changed in the same way. (Obviously not if only the prefix changed.) I mean this could be added with a --view switch.

…d --view and update output messages Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

jochenklar · 2024-06-06T15:37:56Z

rdmo/management/management/commands/merge_attributes.py

@@ -26,16 +26,24 @@ def add_arguments(self, parser):
            help='The URI of the target attribute that will be used to replace the source'
        )
        parser.add_argument(
-            '--save',
+            '--delete',


Why both with the same argument, I can see a case where I want to move everthing from one attribute to another one, but keep the old. Does that make sense?

ok, the args are now:

--source SOURCE The URI of the source attribute that will be replaced by the target and will be deleted --target TARGET The URI of the target attribute that will be used to replace the source --save If specified, the changes will be saved. If not specified, the command will not make any changes in the database. --delete If specified, the source attribute will be deleted. If not specified, the command will not delete the source attribute. --view If specified, the changes will be applied to the affected Views as well.

…elete and --view and update tests and output messages Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

MyPyDavid added type:feature-request status:acceptance labels May 14, 2024

MyPyDavid added this to the RDMO 2.2.0 milestone May 14, 2024

MyPyDavid self-assigned this May 14, 2024

MyPyDavid changed the title ~~Merge domain attributes via CLI command~~ Move related items from one attribute to another via CLI command May 14, 2024

MyPyDavid linked an issue May 14, 2024 that may be closed by this pull request

Maintenance of attributes in an instance, functionality for moving all related items from one attribute to another #990

Open

MyPyDavid marked this pull request as ready for review May 15, 2024 09:54

jochenklar reviewed May 17, 2024

View reviewed changes

rdmo/management/management/commands/merge_attributes.py Outdated Show resolved Hide resolved

MyPyDavid marked this pull request as draft May 17, 2024 09:55

MyPyDavid added the status:work-in-progress label May 17, 2024

jochenklar requested changes May 17, 2024

View reviewed changes

MyPyDavid removed the status:acceptance label May 21, 2024

MyPyDavid added 3 commits May 21, 2024 12:28

feat: add merge attribute command

b7e23bf

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

chore: refactor merge_attributes into procedural style

44be7e0

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

chore: refactor merge attributes command for #990

4c24589

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

MyPyDavid force-pushed the feat-merge-domain-attributes branch from 09a7fe3 to 4c24589 Compare May 21, 2024 10:29

MyPyDavid marked this pull request as ready for review May 21, 2024 13:15

MyPyDavid requested a review from jochenklar May 21, 2024 13:15

MyPyDavid removed the status:work-in-progress label May 23, 2024

jochenklar approved these changes May 30, 2024

View reviewed changes

jochenklar reviewed May 30, 2024

View reviewed changes

MyPyDavid added the status:work-in-progress label Jun 4, 2024

MyPyDavid changed the title ~~Move related items from one attribute to another via CLI command~~ Move related items from one attribute to another via CLI command (merge_attributes) Jun 6, 2024

MyPyDavid added 2 commits June 6, 2024 15:34

feat(management command): change merge_attributes args to --delete an…

15ecb14

…d --view and update output messages Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

tests(management): update args for merge_attribute command tests

878d4e9

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

MyPyDavid requested a review from jochenklar June 6, 2024 14:30

jochenklar reviewed Jun 6, 2024

View reviewed changes

feat(management command): change merge_attributes args to --save, --d…

c6a519d

…elete and --view and update tests and output messages Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

MyPyDavid requested a review from jochenklar June 7, 2024 12:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move related items from one attribute to another via CLI command (merge_attributes) #991

Move related items from one attribute to another via CLI command (merge_attributes) #991

MyPyDavid commented May 14, 2024 •

edited

MyPyDavid commented May 15, 2024

jochenklar commented May 15, 2024

jochenklar May 17, 2024

jochenklar May 17, 2024

MyPyDavid May 21, 2024

jochenklar May 17, 2024

MyPyDavid May 21, 2024

jochenklar May 17, 2024

MyPyDavid May 21, 2024

jochenklar May 17, 2024

MyPyDavid May 21, 2024

jochenklar May 30, 2024

jochenklar May 30, 2024

MyPyDavid Jun 3, 2024 •

edited

jochenklar Jun 4, 2024

MyPyDavid Jun 4, 2024

jochenklar Jun 4, 2024

jochenklar Jun 4, 2024

MyPyDavid Jun 6, 2024

jochenklar Jun 6, 2024

MyPyDavid Jun 6, 2024

MyPyDavid Jun 6, 2024 •

edited

MyPyDavid commented Jun 6, 2024

jochenklar commented Jun 6, 2024

jochenklar Jun 6, 2024

MyPyDavid Jun 7, 2024

		return replace_attribute_on_elements(source, target, REPLACE_ATTRIBUTE_MODEL_HELPERS, save)


		class Command(BaseCommand):

		return msg


		def replace_attribute_on_elements(source, target, model_helpers_with_attributes, save):


		ModelHelper = namedtuple('ModelHelper', ['model', 'lookup_field', 'field_introspection', 'replacement_function'])

		REPLACE_ATTRIBUTE_MODEL_HELPERS = [

		return instance


		def replace_attribute_on_element_instances(model,

Move related items from one attribute to another via CLI command (merge_attributes) #991

Are you sure you want to change the base?

Move related items from one attribute to another via CLI command (merge_attributes) #991

Conversation

MyPyDavid commented May 14, 2024 • edited

Description

Motivation and Context

How has this been tested?

Screenshots (if appropriate)

Types of Changes

Checklist

MyPyDavid commented May 15, 2024

jochenklar commented May 15, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MyPyDavid Jun 3, 2024 • edited

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 Jun 6, 2024 • edited

Choose a reason for hiding this comment

MyPyDavid commented Jun 6, 2024

jochenklar commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MyPyDavid commented May 14, 2024 •

edited

MyPyDavid Jun 3, 2024 •

edited

MyPyDavid Jun 6, 2024 •

edited