Replace SnippetObjectType with SnippetInterface #405
Conversation
The name `CustomInterface` is too similar with `CustomPageInterface` and the upcoming `CustomSnippetInterface` and their custom interfaces settings file. Better rename it to `AdditionalInterface` so there is less confusion.
The settings file will contain an override for the default snippet interface, in addition to the default page interface, so we're renaming it up front here.
Fixes torchbox#386 API clients could use a field on snippet objects to determine the type of snippet they are looking at. Therefore, we change the snippet type to an interface, similar to the page interface, so it can expose a new field called `snippetType`.
mgax
force-pushed
the
feature/snippet-interface
branch
from
29daccb
to
e2729f4
Compare
| @@ -0,0 +1,126 @@ | |||
| # Generated by Django 5.0.9 on 2024-09-20 07:17 | |||
There was a problem hiding this comment.
note to self: regenerate migrations so we're back to just the initial one
There was a problem hiding this comment.
Ah, I can do that in this PR.
There was a problem hiding this comment.
I've folded migration 0003 into 0001, though I'm now wondering why it was generated, and why the wagtail.fields.StreamField constructor arguments have changed.
| ( | ||
| "heading", | ||
| wagtail.blocks.CharBlock(form_classname="full title"), | ||
| ("heading", 0), |
There was a problem hiding this comment.
this looks like the Wagtail 6.2 changes - https://docs.wagtail.org/en/latest/releases/6.2.html#compact-streamfield-representation-for-migrations
What version did you run the migrations with? It doesn't matter as such because they are more logical representations, and we don't do anything with SF migration
There was a problem hiding this comment.
Yep, I've generated them with Wagtail 6.2. But the test matrix is passing, so it looks like older Wagtail versions can still read them.
|
Nice work, the changes make sense to me. I think it would be nice to have some tests for:
to make sure there are no surprises there. |
@jams2 I've added a test which stubs out the registered snippets, to test the scenario when there are no registered snippets. The test app already has two snippet types, and I've updated the initial test, to include one snippet of each type, to make sure they get returned in the same result set with no issues. |
|
@mgax looks good, thanks. |
Fixes #386
API clients could use a field on snippet objects to determine the type of snippet they are looking at. Therefore, we change the snippet type to an interface, similar to the page interface, so it can expose a new field called
snippetType.This PR includes a couple of refactoring commits which might be easier to view separately:
CustomInterfaceis too similar withCustomPageInterfaceand the upcomingCustomSnippetInterfaceand their custom interfaces settings file. Better rename it toAdditionalInterfaceso there is less confusion.