Re-implement the tag editing component

[ErisDS]

The tag editing component is one of the more complex pieces of the Ghost admin interface, and it was largely 'ported' to Ember, as opposed to rebuilt in a more ember-y fashion. Unfortunately, it is largely broken, particularly on mobile, and needs a bit of a rethink.

The component consists of two main pieces: 1) the suggestions popover, which uses our ember popover component and I believe is working well across all devices, and 2) the tag input, which really needs some love. Any attempt to fix bugs with the current implementation promptly results in a game of whack-a-mole as other things break. I believe it needs rethinking and rebuilding from scratch.

This issue exists as a statement of intent that we will rebuild this, either on it's own if someone feels like taking it on, or possibly as part of the work on a new editor. All known bugs with this component are going to be closed as wontfix against this issue, pending the rewrite.

[ErisDS]

This tag component will be moving to the PSM at some point in the future. It's also a horrible, horrible mess and needs rewriting to be a more independent component.

One of the major things I'd like to change, is to have the tag component manage the tag models itself. Rather than sending off all of the tags with the post to be saved, I think it is cleaner, safer and more restful to save the tags independently when they are updated.

To do this, I need to add new API endpoints, as we don't just need CRUD on a tag, but on a tag for a post. The restful way to do this would be to use a nested resource:

POST /posts/:id/tag/

Is different from

POST /tag/

In that it should create a relationship between the given tag and the given post, as well as creating the tag if it doesn't exist yet.

I did a bit of research into how to do this with EmberData, given that this is quite a complex concept, and turned up a lot of information which said that nested resources were not supported. In a conversation with @novaugust he also expressed a concern of how EmberData was meant to know the difference between creating a tag, and creating a tag for a specific post.

I turned up this solution, which it seems reasonably sane especially considering that we do want both the concepts of creating a tag and creating a tag & relating it to a post to exist in the admin UI:

http://discuss.emberjs.com/t/current-way-to-handle-nested-resources/7477/4

I'm keen to get started speccing the issues to create the new API endpoints, so that the ember work can be done, but before I do I want to check that this is definitely a feasible and sensible path to go down, or if there are other / better suggestions.

@novaugust @jaswilli @rwjblue your advise would be most welcome here!

[novaugust]

I feel weird having two endpoints for making tags. I'd love if we could
keep it at just one endpoint, but include information on the tag if it's to
be linked to a post. So, always ping /tag, but just include an optional
post id with it if it's for a post. Meow?

On Sat, Apr 11, 2015 at 10:03 AM, Hannah Wolfe [email protected]
wrote:

This tag component will be moving to the PSM at some point in the future.
It's also a horrible, horrible mess and needs rewriting to be a more
independent component.

One of the major things I'd like to change, is to have the tag component
manage the tag models itself. Rather than sending off all of the tags with
the post to be saved, I think it is cleaner, safer and more restful to save
the tags independently when they are updated.

To do this, I need to add new API endpoints, as we don't just need CRUD on
a tag, but on a tag for a post. The restful way to do this would be to use
a nested resource:

POST /posts/:id/tag/

Is different from

POST /tag/

In that it should create a relationship between the given tag and the
given post, as well as creating the tag if it doesn't exist yet.

I did a bit of research into how to do this with EmberData, given that
this is quite a complex concept, and turned up a lot of information which
said that nested resources were not supported. In a conversation with
@novaugust https://github.com/novaugust he also expressed a concern of
how EmberData was meant to know the difference between creating a tag, and
creating a tag for a specific post.

I turned up this solution, which it seems reasonably sane especially
considering that we do want both the concepts of creating a tag and
creating a tag & relating it to a post to exist in the admin UI:

http://discuss.emberjs.com/t/current-way-to-handle-nested-resources/7477/4

I'm keen to get started speccing the issues to create the new API
endpoints, so that the ember work can be done, but before I do I want to
check that this is definitely a feasible and sensible path to go down, or
if there are other / better suggestions.

@novaugust https://github.com/novaugust @jaswilli
https://github.com/jaswilli @rwjblue https://github.com/rwjblue your
advise would be most welcome here!

—
Reply to this email directly or view it on GitHub
#3800 (comment).

[ErisDS]

@novaugust from a wider API design view point, having another API endpoint for tags is correct, I believe, because GET /posts/:id/tags would return all of the tags for a particular post. Which is different to GET /tags/ and different to GET /posts/?include=tags which are the endpoints we have now.

Also sending the post with the tag in order to save, ends us up in a similar position to what we have now where we save all tags with the post, where the model code is playing a guessing game, saving the tag first and then trying to figure out what to do with the relationships. The intention is to make the endpoints much more explicit, if possible.

[ErisDS]

I'd love to get a bit more feedback on this. I'm working on a full spec for what the API should look like and deciding which way to go on this is blocking me a bit. Not sure if I should break it out into a separate discussion issue, I really want to get a bit of clarity on whether or not we can or should support nested resources in the API.

@sebgie I think from an API perspective it does make sense to move to /posts/3/tags/ and similarly /posts/3/next/ instead of /posts/3/?include=next. However from an ember perspective I really need a bit of insight on whether using namespaces will work for us?

Alternatively /tags/?post_id=3 could work as a filter for the tag list, but it doesn't feel right. When you put it in context with the idea that we ideally need to move to nesting for next & previous as well, it starts to look more sensible & consistent, I believe.

From http://discuss.emberjs.com/t/current-way-to-handle-nested-resources/7477/3:

adapter.set('namespace',/posts/${post.id});

Seems sane, although a bit complex.

Perhaps putting a spec up with 2 versions would be easier to frame the conversation?

[novaugust]

I would say build the api you want to have; keep in mind that Ember won't be the only client consuming it, so we shouldn't be around it. We can make ember-data do what it needs to do to get the right endpoints =) Nesting really shouldn't be an issue; what gets weird is having nesting + first-level, but I reckon we could figure it out

[jaswilli]

I don't see any problems with /posts/:id/tags or /posts/:id/next. We can change the tags relationship on the Post model from embedded to the "traditional" array of ids and it never needs to know about /posts/:id/tags.

If the tag editor wants to get/set the tags linked to a post we can build something to do those requests using ajax. Even post- JSON-API 1.0 and Ember-Data 1.0 I don't think there's going to be a way to manipulate a resource's links without calling through that resource, so if we want to do that using ajax outside of Ember Data seems totally reasonable to me.

[sebgie]

@sebgie I think from an API perspective it does make sense to move to /posts/3/tags/ and similarly /posts/3/next/ instead of /posts/3/?include=next.

There is a huge difference between the examples above:

• /posts/3/?include=tags - will return post with id = 3 and include information about associated tags
• /posts/3/tags/- will return all tags that are associated with the post but not the post itself

For the tag editing component I think that we'll need both versions of the endpoint. /posts/3/tags/ is used to get all tags and even more important to save the tags without sending the full post back to the server. /posts/3?include=tags/ for the front end or third party API requests?

keep in mind that Ember won't be the only client consuming it

@novaugust 👍 ❤️

[ErisDS]

All good points, thanks 😍 I feel a bit better now about speccing this out. Watch this space!

[ErisDS]

This is now required as a part of Zelda

[ErisDS]

There are a couple of big parts to getting this done:

• rebuild the component in editor
• update the API to handle tag saving/adding/removing better
• reimplement the suggestions system to use typeahead/whatever gets used in [Zelda] Post/Tag/User auto-complete 'search' bar #5343

I'm thinking I need to write up a bit more of a spec - would be happy to do the API part if someone wanted to do the ember bits with me.

[JohnONolan]

Reference material: It would be great if the new field could work exactly like adding tags to a story on pocket.co

[acburdine]

raises hand for ember part

@ErisDS Once I'm done with the inline-errors stuff, I can take a crack at the ember side of things.

[ErisDS]

Whilst I think about it - one of the key things we want to do when we've got a new version of this component is introduce drag & drop re-ordering, so that's worth keeping in mind for whoever starts re-architecting this.

[acburdine]

jQueryUI has good built in functionality for the drag and drop portion of this here. That would IMO be the easiest to use because it wouldn't require any additional libraries to do.

[ErisDS]

This is effectively closed by #5553 / 2c5d2d6.

#5648 is open for bugfixes / cleanup.