JSON Content Editor

[sarahelsaig]

Parent PR: Lombiq/Open-Source-Orchard-Core-Extensions#634

This PR adds an "Edit as JSON" button to the actions dropdown in the admin content item listing:

Clicking on it encodes the content item as JSON and displays it in the JSON editor:

This still requires write access to the content item so the security is the same as the regular content item editor. Clicking "Publish" will deserialize the received JSON into a ContentItem and saves it into the database.

The use-cases for this editor:

• Inspect the raw content item JSON without having to export the content or directly access the backing YesSql database
• Edit fields that normally don't have an editor, e.g. Author, DisplayText, set arbitrary creation date, etc.

[Piedone]

Thank you for your contribution! BTW I think this would be great to demo at the Orchard meeting.

[Piedone]

Something similar should happen with this too:

existing = await _contentManager.GetAsync(contentItem.ContentItemId, VersionOptions.DraftRequired);
...
await _contentManager.PublishAsync(contentItem);

[sarahelsaig]

This is not good. If the content is versionable, this creates a new draft clone of the content item we have no use for. We don't want that, because the new version's content should come from the JSON, not through cloning the existing item.

[Piedone]

When the content is versionable, we need to edit a new or existing draft, not the latest version, what might be a published one. Unless you want to let people circumvent versioning, what I wouldn't do.

[sarahelsaig]

We are not editing the latest version. We are unpublishing it and the publishing a new content item that's deserialized from the received JSON.

[Piedone]

Why is this necessary instead of utilizing Orchard's versioning?

[jtkech]

Just for info and from memory

• With STJ we did a JConvert helper to serialize /deserialize a content item as before.

  JConvert.DeserializeObject<ContentItem>(json)
  

• Yes the Version column is not related to the ContentItemVersionId, it is used when we save an item with checkConcurrency: true, it throws if a given record is written concurrently.

• Yes this is not obvious to call the right content manager methods and in the right order ;)

  First I assume that the editor doesn't allow to modify some data that are still managed internally, like ContentItemId, ContentItemVersionId, Latest, Published and so on. And I saw that from the editor we only can publish, not save as a draft.

  Why not if it is an advanced feature intended to act directly on a given version record, but yes here we don't follow the version management pattern, e.g. loading / activating a new version if the type is Versionable. Yes, the Latest may not be Published, hmm but if this is the one we are editing it will be, if an item is already Published, PublishAsync() does nothing, that why I think you needed to call _session.Save().

  Okay there is no driver validations but as I can see no updating and validating handlers will be called too. So what I suggest is to follow the pattern used in the ApiController.Post of the OC.Contents module.

  It requests to load and activate a DraftRequired, a new version is built if the type is Versionable, the result being unpublished, if it doesn't exists here you can return NotFound, then it merges this existing item version with the provided one, here can be deserialized from the editor, then it calls UpdateAsync() and ValidateAsync() that calls validating handlers, finally it does SaveDraftAsync() or as here PublishAsync() (here the item is not already published) that may unpublish a previous version.

  The whole without having to call any _session.Save().

[Piedone]

Thank you for chiming in, JT. Reopening this convo, then.

[sarahelsaig]

With STJ we did a JConvert helper to serialize /deserialize a content item as before.

I see it in your branch but it's not in OC proper yet, right? It's a great idea and will make porting easier for everyone.

First I assume that the editor doesn't allow to modify some data that are still managed internally, like ContentItemId, ContentItemVersionId, Latest, Published and so on.

You can edit the ContentItemId, this could be used to clone the current item by publishing it with a different ID. ContentItemVersionId, Latest, and Published are overwritten to values you'd expect from a Publish type action.

And I saw that from the editor we only can publish, not save as a draft. Why not if it is an advanced feature intended to act directly on a given version record

You can always turn the content item into a draft using the stock actions menu after it's saved. I almost never use drafts so I had no interest adding this feature at the moment. (my interest is only relevant here because I made this contrib on my own initiative, outside of work) If there is any demand for saving a draft, the editor can be expanded later without breaking changes.

if an item is already Published, PublishAsync() does nothing, that why I think you needed to call _session.Save().

In the current version I just set the contentItem.Published to false before calling PublishAshync to force it, instead of calling _session.Save(). But that's delving int implementation detail, I think it would be better if there was an additional optional parameter on the method like IContentManager.PublishAsync(ContentItem contentItem, bool forcePublish = false). What do you think? If no problem with it comes to mind, I may submit a PR for that in OC.

So what I suggest is to follow the pattern used in the ApiController.Post of the OC.Contents module.

So would it make more sense to ditch the highlighted code and just pass the content item to ApiController.Post directly? That would be a lot of code to copy/reimplement...

[sarahelsaig]

I tried just passing the content item to ApiController.Post and it works great! Now that it's trivial to handle drafts by just passing true to the second parameter, I've added that too.
The only tricky part was that this action requires Permissions.AccessContentApi permission, but it's not so hard to grant that to a temporary claims principal, so I did that.

[jtkech]

Okay I understand the context so feel free to do whatever you think is better, these are only suggestions.

• We already have a dedicated converter allowing to serialize / deserialize a content item, you are using it here and it will be the same with STJ. Hmm maybe in the controller action you could directly bind to a content item as done in the Api controller action.

• Okay I understand, it is more like an advanced feature allowing to update a given version record without having to deal with validations, draft versions and so on. I was not sure but yes in that case my suggestions are less relevant and maybe what you first did was better.

• Good idea to use the Api action but I would not recommend this pattern, I think it is better to keep a dedicated controller action that would be easier to tweak.

• Otherwise, following my first bad idea, once you have a deserialized content item, could be as simple as the following.

  ...
  var contentItem = await _contentManager.GetAsync(
      deserialized.ContentItemId,
      VersionOptions.DraftRequired);
  if (contentItem is null)
  {
      return NotFound();
  }
  
  if (!await _authorizationService.AuthorizeAsync(
      User,
      CommonPermissions.EditContent,
      contentItem))
  {
      return Forbid();
  }
  
  contentItem.Merge(deserialized, _updateJsonMergeSettings);
  await _contentManager.UpdateAsync(contentItem);
  
  var result = await _contentManager.ValidateAsync(contentItem);
  if (!result.Succeeded)
  {
      // Whatever is better to do.
  }
  
  await _contentManager.PublishAsync(contentItem);
  ...
  

[jtkech]

@sarahelsaig @Piedone Just in case you missed the following comment I did but in a resolved section.

---

Just for info and from memory

• With STJ we did a JConvert helper to serialize /deserialize a content item as before.

  JConvert.DeserializeObject<ContentItem>(json)
  

• Yes the Version column is not related to the ContentItemVersionId, it is used when we save an item with checkConcurrency: true, it throws if a given record is written concurrently.

• Yes this is not obvious to call the right content manager methods and in the right order ;)

  First I assume that the editor doesn't allow to modify some data that are still managed internally, like ContentItemId, ContentItemVersionId, Latest, Published and so on. And I saw that from the editor we only can publish, not save as a draft.

  Why not if it is an advanced feature intended to act directly on a given version record, but yes here we don't follow the version management pattern, e.g. loading / activating a new version if the type is Versionable. Yes, the Latest may not be Published, hmm but if this is the one we are editing it will be, if an item is already Published, PublishAsync() does nothing, that why I think you needed to call _session.Save(). Okay there is no driver validations but as I can see no updating and validating handlers will be called too.

• So what I suggest is to follow the pattern used in the ApiController.Post of the OC.Contents module.

  It requests to load and activate a DraftRequired, a new version is built if the type is Versionable, the result being unpublished, if it doesn't exists here you can return NotFound, then it merges this existing item version with the provided one, here can be deserialized from the editor, then it calls UpdateAsync() and ValidateAsync() that calls validating handlers, finally it does SaveDraftAsync() or as here PublishAsync() (here the item is not already published) that may unpublish a previous version.

  The whole without having to call any _session.Save().

[Piedone]

Just to make sure: are you done and waiting for a review here? (Since you didn't re-request review.)

[sarahelsaig]

Yes, I'm done. Sorry.

[Piedone]

Thank you!