-
Notifications
You must be signed in to change notification settings - Fork 9
Add initial snippets & service page model, expose API #11
Conversation
Implements parts of #2, #3, and #6 Proof is in the pudding: ```shell curl http://localhost:8000/api/v2/pages/\?format\=json\&type\=base.ServicePage\&fields\=content,extra_content,tags | jq ``` ```json { "meta": { "total_count": 1 }, "items": [ { "id": 4, "meta": { "type": "base.ServicePage", "detail_url": "http://localhost/api/v2/pages/4/", "html_url": "http://localhost/sweet-new-page/", "slug": "sweet-new-page", "first_published_at": "2017-11-28T19:47:05.575059Z" }, "title": "This is a sweet new page", "content": "<p>This is some sweet content</p><p><br/></p><ol><li>do this first</li><li>then do this</li><li>now you're done<br/></li></ol>", "extra_content": [ { "type": "content", "value": "<p><b>This is my extra content</b><br/></p>", "id": "695e363a-6ecf-457e-8e53-e351aecc2ad6" }, { "type": "application_block", "value": 1, "id": "dbb9e30f-2aff-4669-aee3-e555192a8580" }, { "type": "content", "value": "<h1>This is some other extra content<br/></h1>", "id": "fb4ab425-9391-496a-aa5f-70915b02bde5" } ], "tags": [ "nolo", "rolo", "yolo" ] } ] } ```
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.
This python stuff is v new to me. Hoping you get @briaguya to give you more feedback on this stuff if needed. But I'll give it a
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), | ||
('name', models.CharField(max_length=255)), | ||
('photo', models.CharField(max_length=255)), | ||
('email', models.EmailField(max_length=254)), |
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.
don't think this matters at all but why is this magic number different from the rest?
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.
You have to manually specifyCharField.max_length
and 255 is a pretty common default. Django chooses a sensible default for EmailField.max_length
which is apparently 254 :|
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.
These migrations are auto-generated from models, so I'll probably punt on doing that here. I'll add some constants to models.py
so magic numbers aren't scattered everywhere.
|
||
|
||
class HomePage(Page): | ||
pass | ||
|
||
|
||
WYSIWYG_FEATURES = ['h1', 'h2', 'h3', 'bold', 'italic', 'link', 'ul', 'ol'] |
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.
👍 Are there any other options in this list you considered or that we may want to include in the future?
Also wondering if there will be any sanitization or parsing required when passing a string of html to the frontend via the API. There must be a trick to get it to render as HTML. Maybe we'll have to use the dangerouslySetInnerHtml
method in React: https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
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.
The API responses come back as HTML currently but it's fairly simple to modify if there's a better representation to use.
There are other options available listed in the docs but these seemed like most of what an author would need.
created_at = models.DateTimeField(auto_now_add=True) | ||
updated_at = models.DateTimeField(auto_now=True) | ||
|
||
content = RichTextField(features=WYSIWYG_FEATURES, help_text='Write out the steps a resident needs to take to use the service') |
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.
🆒
def __str__(self): | ||
return self.description | ||
|
||
|
||
@register_snippet |
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.
what does this do? what is this syntax?
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.
It's a python decorator, which is used to modify a class/function somehow. In this case, wagtail uses it as a way to mark these classes as snippets so they can be used in the admin. Here's more on what wagtail snippets actually are http://docs.wagtail.io/en/latest/topics/snippets.html
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.
👍
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.
- Use constants instead of magic numbers for `max_length` - Pin `django-cors-headers` dependency to specific version
Implements parts of #2, #3, and #6
Proof is in the pudding: