cortex-cms / cortex

:pencil: A headless, multitenant dynamic content platform powered by Rails, GraphQL and Elasticsearch
https://docs.cortexcms.org/
Apache License 2.0
32 stars 6 forks source link

ContentItem FieldType #423

Closed toastercup closed 7 years ago

toastercup commented 7 years ago

This is a proposal for a FieldType that will allow a content creator to associate one ContentItem with another ContentItem. There's a few requirements that make this feature tricky and non-obvious:

With all of these requirements in mind, the solution I'm proposing should be accommodating. When defining a Field, you'll be able to pass in some configuration in the metadata field:

blog.fields.new(name: 'Tile Image', field_type: 'content_item_field_type',
                metadata: {
                  contract_id: 1, # How to render Selection Index?
                  field_id: 1, # How to present Selected ContentItem?
                  queries: [ # How to filter Selection Index?
                    {
                      filter_by_asset_content_type: ['image/png']
                    }
                  ]
                })

There's no need to specify the ContentType, as that's implied by the field_id's parent. queries is a sorted array that specifies queries to be executed, in the sorted order, to filter or sort the data to be presented in the specified Contract. This utilizes the yet-to-be-built Dynamic Query system, which will build ElasticSearch queries on-the-fly using query builder helpers contained within FieldType plugins.

To display the selected ContentItem in the creation interface, we need a way to represent the FieldType associated with the field_id specified in the metadata config hash for the ContentItemField. To accomplish this, I feel there should be an interface all FieldTypes can conform to to implement this feature. By exposing a public render method, association, a FieldType's Cell can help present information to the user about the selected ContentItem.

ElliottAYoung commented 7 years ago

@toastercup I like the idea of tracking the specific field, I feel like it makes it cleaner and more consistent. I would advocate to change queries: to something else, I get the feeling like that isn't the clearest name for this, however I can't really think of a better one off the top of my head. The inclusion of contract is also a good call, an entirely necessary good call, but the difficulty there is we're going to have to build in some way to force all ContentTypes into at least one contract at creation - this shouldn't be too hard, just something we have to do.

I had a similar thought for doing asset type filtering and think that using the mime types is probably our best bet as well, it will also allow us to more easily filter on the present assets in the contract given the existing_data hash we're tracking. Small thing to note: we're going to need to track a render_method on this hash as well, but that's about as minimal as it gets.

Quick question on all of this - would we want to plan to use one system for both ContentItemFieldType pairing selection and WYSIWYG image inclusion, or continue to have them separate? My reasons for asking have to do with the contract inclusion and specification to one field - it could get confusing what needs to be referenced when the WYSIWYG popup is triggered, we would essentially need to track this same metadata when creating the WYSIWYG field type for that to work then...which again, isn't the worst thing, I just want to voice it and think through it here...

ElliottAYoung commented 7 years ago

Overall - I'm 100% fine proceeding with this how it is now, I think we cover all of our bases and it leaves us room to grow into the structure should new needs arise :+1:

MKwenhua commented 7 years ago

Sounds good 👍 , but I was thinking that including content_type couldn't hurt(even if kind of redundant). Sure ContentType can always be inferred by the field_id , but maybe be more obvious to the reader with an explicit name. I would be fine without it though, but anyways I think it's a solid solution!

toastercup commented 7 years ago

@ElliottAYoung Yarp, I'm cool with finding a better name for queries. We can kick that can down the road, since we're not implementing this bit for now.

As for the contract association enforcement - this shouldn't be necessary. A Contract needn't include any ContentTypes to successfully render - it'll just return zero results, but shouldn't necessarily break. I think I understand your intention, but we might also be out of sync - going back to our ever-so-slight differences in implementation for the Contract system.

What would the render_method you suggest we add to this hash be used for? We usually specify those at the Decorator level for every Field that's rendered.

Lastly, I don't think the WYSIWYG ContentItem reference system could share much code with this system (though it may share similar configuration conventions). I envision some sort of object within the data hash for a TextFieldItem that stores tracking information concerning any shortcodes used within the actual text data, rather than using another field to store this tracking data (which would require extra intervention on a superadministrator's part, to remember to set this field up so tracking data can be stored). Configuration for setting up the desired Contract and such would look very similar to what I've laid out here, but would just be a bit of configuration in the Decorator (since that's where WYSIWYG is enabled, configured, plugins are setup, etc), rather than at the Field level.

@MKwenhua I pondered more regarding my decision not to specify content_type_id. The primary reason is that a Contract can contain multiple ContentTypes within its index, and advanced use cases of a ContentItemFieldType may want to support this notion. For example, there may be multiple ways, in a tenant's system, to represent Media. CareerBuilder's tenant, for example, may end up with 2 Media ContentTypes - one that contains metadata specific to Employer, and the other that's specific to Jobseekers. What if we wanted a unified resource center with blogposts that could refer to these Media through one generic ContentItemFieldType reference? Enforcing one ContentType for association would limit this kind of flexibility (which is the whole point of this high-falutin' system), but so does my current proposal of specifying the Field! Therefore, the solution here is to infer the desired Field by specifying a field_name to search for on the selected ContentItem's ContentType when rendering selection information. To this end, I want to introduce a change to the name database field on Field that will track a uniquely identifiable (scoped to a single ContentType), text-based ID that is agnostic of its display label (which will now be specified - and required - in the Decorator). This would mean that if 2 ContentTypes adhered to a similar interface (that is, they both contained a Field with the same name), the system would be able to intelligently render information in both scenarios. If no matching Field was found, the system might default to rendering just the ContentItem's id, and the entire ContentItem when presenting via the API. The new Field may look something like this:

blog.fields.new(name: 'Tile Image', field_type: 'content_item_field_type',
                metadata: {
                  contract_id: 1, # How to render Selection Index?
                  field_name: 'asset', # How to present Selected ContentItem?
                  queries: [ # How to filter Selection Index?
                    {
                      filter_by_asset_content_type: ['image/png']
                    }
                  ]
                })

Thanks! Let me know if this covers all of your concerns.

ElliottAYoung commented 7 years ago

I would prefer tracking the associate field by id rather than name, solely because the field_name is subject to change at any point, just by the very nature of the system. Plus the field_name isn't necessarily unique within the contract and will cause reference errors when we try to implement this

toastercup commented 7 years ago

(See previous comment for more details): field_name not being unique within the contract is the whole point of tracking it in this way. field_name shouldn't be changing willy-nilly - it's not the same thing as a title or label anymore (though it's okay if it does change - nothing will break catastrophically - just the rendering of the selected ContentItem). Instead, the field name is now acting as it always should have been - a unique (scoped) name for the Field record, and not a presentation concern. As long as a superadmin follows the rules for the system, things should work normally and there should be no reference errors. If we track by ID, we will defy the purpose of the system we built (see above comment for more explanation). I'm open to other suggestions, but tracking by ID may be off the table, as far as I can discern. The only other thought I had was to track an array of potential field IDs, and the system will go through each until one matches the ContentType of the associated ContentItem, but to me, this seems very tedious for the superadmin to manage.

toastercup commented 7 years ago

Implemented by #426 and cortex-cms/cortex-plugins-core#32