Skip to content
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

ConfigContext data migration and make sure the SiteLocationType.content_types is set to allow all Location ContentTypes #3222

Merged
merged 3 commits into from
Feb 3, 2023

Conversation

HanlinMiao
Copy link
Contributor

@HanlinMiao HanlinMiao commented Feb 3, 2023

Closes: N/A

What's Changed

  1. Migrate ConfigContext regions and sites data to ConfigContext.locations.
  2. Ensure the new "Site" LocationType allows all ContentTypes allowed by Location (FeatureQuery("locations").get_query()).
  3. Tested thoroughly and did what it was intended.

TODO

  • Explanation of Change(s)
  • Added change log fragment(s) (for more information see the documentation)
  • Attached Screenshots, Payload Example
  • Unit, Integration Tests
  • Documentation Updates (when adding/changing features)
  • Example Plugin Updates (when adding/changing features)
  • Outline Remaining Work, Constraints from Design

@HanlinMiao HanlinMiao changed the title configcontext data migration and make sure the SiteLocationType.content_types is set ConfigContext data migration and make sure the SiteLocationType.content_types is set to allow all Location ContentTypes Feb 3, 2023
Comment on lines 186 to 188
original_locs = list(cc.locations.all())
locations = region_locs + original_locs
cc.locations.set(locations)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think?

Suggested change
original_locs = list(cc.locations.all())
locations = region_locs + original_locs
cc.locations.set(locations)
cc.locations.add(region_locs)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.add() does not work well with lists or querysets apparently.

>>> ConfigContext.objects.first()
<ConfigContext: 123>
>>> c = ConfigContext.objects.first()
>>> c.locations.add(Location.objects.all()
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/django/db/models/fields/__init__.py", line 2434, in to_python
    return uuid.UUID(**{input_form: value})
  File "/usr/local/lib/python3.7/uuid.py", line 157, in __init__
    hex = hex.replace('urn:', '').replace('uuid:', '')
AttributeError: 'LocationQuerySet' object has no attribute 'replace'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/local/lib/python3.7/site-packages/django/db/models/fields/related_descriptors.py", line 952, in add
    through_defaults=through_defaults,
  File "/usr/local/lib/python3.7/site-packages/django/db/models/fields/related_descriptors.py", line 1126, in _add_items
    target_ids = self._get_target_ids(target_field_name, objs)
  File "/usr/local/lib/python3.7/site-packages/django/db/models/fields/related_descriptors.py", line 1073, in _get_target_ids
    target_ids.add(target_field.get_prep_value(obj))
  File "/usr/local/lib/python3.7/site-packages/django/db/models/fields/related.py", line 977, in get_prep_value
    return self.target_field.get_prep_value(value)
  File "/usr/local/lib/python3.7/site-packages/django/db/models/fields/__init__.py", line 2418, in get_prep_value
    return self.to_python(value)
  File "/usr/local/lib/python3.7/site-packages/django/db/models/fields/__init__.py", line 2439, in to_python
    params={'value': value},
django.core.exceptions.ValidationError: ['“<LocationQuerySet [<Location: Campus-00>, <Location: Building-01>, <Location: Elevator-02>, <Location: Elevator-09>, <Location: Floor-03>, <Location: Room-04>, <Location: Aisle-05>, <Location: Aisle-12>, <Location: Room-11>, <Location: Aisle-19>, <Location: Campus-07>, <Location: Building-08>, <Location: Floor-10>, <Location: Room-18>, <Location: Campus-14>, <Location: Building-15>, <Location: Elevator-16>, <Location: Floor-17>, <Location: Root-06>, <Location: Root-13>]>” is not a valid UUID.']
>>> c.locations.add([Location.objects.all()])
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/django/db/models/fields/__init__.py", line 2434, in to_python
    return uuid.UUID(**{input_form: value})
  File "/usr/local/lib/python3.7/uuid.py", line 157, in __init__
    hex = hex.replace('urn:', '').replace('uuid:', '')
AttributeError: 'list' object has no attribute 'replace'

During handling of the above exception, another exception occurred:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to unpack the list e.g. cc.locations.add(*region_locs)

Comment on lines 216 to 218
original_locs = list(cc.locations.all())
locations = site_locs + original_locs
cc.locations.set(site_locs + original_locs)
Copy link
Contributor

@glennmatthews glennmatthews Feb 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly?

Suggested change
original_locs = list(cc.locations.all())
locations = site_locs + original_locs
cc.locations.set(site_locs + original_locs)
cc.locations.add(*site_locs)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ I edited your suggestion to unpack the list.

@HanlinMiao
Copy link
Contributor Author

See #3208 (comment) for more migrations we need to do in this PR

Hanlin Miao added 2 commits February 3, 2023 12:18
…egions/Sites and the number of Region/Site LocationType locations in ConfigContext
@HanlinMiao HanlinMiao merged commit 8fc08d0 into next Feb 3, 2023
@bryanculver bryanculver deleted the hanlin-addition-region-site-data-migration branch February 14, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants