Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Add models to persist embed config #1304

Merged
merged 2 commits into from Apr 13, 2021
Merged

Add models to persist embed config #1304

merged 2 commits into from Apr 13, 2021

Conversation

TaiWilkin
Copy link
Contributor

@TaiWilkin TaiWilkin commented Apr 7, 2021

Overview

The embed config is complex and will grow over time as we add features. Rather than serialize to the query string, store the config in the database and link it to the contributor.

  • Adds configuration models
  • Adds creation, update and fetching of embedded map configurations via API
    • Note that the nested embedded map fields are deleted and recreated on update of the parent EmbedConfig
  • Adds embedded map configuration to the data fetched for user profiles

Connects #1299

Demo

Screen Shot 2021-04-06 at 11 58 20 AM

Testing Instructions

  • Run ./scripts/manage migrate
  • Run ./scripts/server and login as a user with an associated contributor
  • Navigate to :8081/api/embed-configs/
  • POST a new embed config object
    • The new EmbedConfig should be created successfully
     {
	"width": "100",
	"height": "100",
	"color": "pink",
	"font": "serif",
	"show_other_contributor_information": false,
	"embed_fields": [{
		"column_name": "xyz",
		"display_name": "123",
		"visible": true,
		"order": 1
	}]
   }
  • Navigate to [your profile](http://localhost:6543/profile/8]
    • Confirm in the network tab that the EmbedConfig is returned with the contributor details
  • Navigate to your new EmbedConfig and update some fields, including the embed_fields
    • The configuration should update successfully
  • Attempt to create an embed_field with a duplicate order
    • You should receive an error message
  • Attempt to create a new EmbedConfig with the same user
    • You should receive an error message

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

@TaiWilkin TaiWilkin requested a review from jwalgran April 7, 2021 19:15
@TaiWilkin TaiWilkin changed the base branch from develop to tw/setup-tabbed-profile April 7, 2021 20:47
@jwalgran jwalgran added the task 004 Enable contributors to embed a map displaying only the facilities with which they are associated in label Apr 8, 2021
Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

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

The models look good but I found what look like a few required fixes that we need to make to the API.

src/django/api/views.py Outdated Show resolved Hide resolved
src/django/oar/urls.py Outdated Show resolved Hide resolved
src/django/api/views.py Outdated Show resolved Hide resolved
src/django/api/views.py Show resolved Hide resolved
src/django/api/views.py Show resolved Hide resolved
Base automatically changed from tw/setup-tabbed-profile to develop April 9, 2021 15:22
@TaiWilkin TaiWilkin requested a review from jwalgran April 12, 2021 13:54
Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

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

I tested the permission updates and they look good. There were two lines in the test instructions that failed for me

  • Preventing the creation of multiple configs for a single contributor
  • Returning the embed config from the profile endpoint

Consider changes similar to these:

diff --git a/src/django/api/serializers.py b/src/django/api/serializers.py
index fd9bab1..a9ee624 100644
--- a/src/django/api/serializers.py
+++ b/src/django/api/serializers.py
@@ -171,11 +171,13 @@ class UserProfileSerializer(ModelSerializer):
     other_contributor_type = SerializerMethodField()
     facility_lists = SerializerMethodField()
     is_verified = SerializerMethodField()
+    embed_config = SerializerMethodField()
 
     class Meta:
         model = User
         fields = ('id', 'name', 'description', 'website', 'contributor_type',
-                  'other_contributor_type', 'facility_lists', 'is_verified')
+                  'other_contributor_type', 'facility_lists', 'is_verified',
+                  'embed_config')
 
     def get_name(self, user):
         try:
@@ -233,6 +235,12 @@ class UserProfileSerializer(ModelSerializer):
         except Contributor.DoesNotExist:
             return False
 
+    def get_embed_config(self, user):
+        try:
+            return EmbedConfigSerializer(user.contributor.embed_config).data
+        except Contributor.DoesNotExist:
+            return None
+
 
 class FacilityListSummarySerializer(ModelSerializer):
     contributor_id = SerializerMethodField()
diff --git a/src/django/api/views.py b/src/django/api/views.py
index 5c60122..e5aac11 100644
--- a/src/django/api/views.py
+++ b/src/django/api/views.py
@@ -3534,6 +3534,8 @@ class EmbedConfigViewSet(mixins.ListModelMixin,
             return Response(status=status.HTTP_401_UNAUTHORIZED)
 
         contributor = self.get_contributor(request)
+        if contributor.embed_config is not None:
+            raise ValidationError('Contributor has an existing embed configuration.')
 
         fields_data = request.data.pop('embed_fields', [])
 

@@ -3531,7 +3534,7 @@ def create(self, request):
raise ValidationError(
'Contributor has an existing embedded map configuration.')

fields_data = request.data.pop('embed_fields')
fields_data = request.data.pop('embed_fields', [])
Copy link
Contributor

Choose a reason for hiding this comment

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

This is nice. I don't regularly use pop and may have missed the fact that you can provide a default argument 👍

@TaiWilkin
Copy link
Contributor Author

Thanks for the feedback! I've updated the profile fetching and the contributor check in d4b0b0c

@TaiWilkin TaiWilkin requested a review from jwalgran April 13, 2021 13:31
Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for working through the changes with me.

@jwalgran jwalgran removed their assignment Apr 13, 2021
The embed config is complex and will grow over time as we add features.
Rather than serialize to the query string, store the config in the
database and link it to the contributor.
Allow users to create, fetch and update embedded map configuration
settings.
@TaiWilkin
Copy link
Contributor Author

Thank you for reviewing!

@TaiWilkin TaiWilkin merged commit ea8c62f into develop Apr 13, 2021
@TaiWilkin TaiWilkin deleted the tw/add-embed-models branch April 13, 2021 20:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
task 004 Enable contributors to embed a map displaying only the facilities with which they are associated in
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants