Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide option to include a revision log message #95

Closed
mjordan opened this issue Apr 12, 2020 · 17 comments · Fixed by #645
Closed

Provide option to include a revision log message #95

mjordan opened this issue Apr 12, 2020 · 17 comments · Fixed by #645
Labels
enhancement New feature or request limitation of dependency A known limitation resulting from another library/application

Comments

@mjordan
Copy link
Owner

mjordan commented Apr 12, 2020

Some repo admins might want to know that an object was created by Islandora Workbench. This structure works for POST:

{
  "type": [
    {
      "target_id": "islandora_object",
      "target_type": "node_type"
    }
  ],
  "title": [
    {
      "value": "Created a collection with POST"
    }
  ],
  "revision_log": [
    {
      "value": "Created by Islandora Workbench."
    }
  ],  
  "field_model": [
    {
      "target_id": 23,
      "target_type": "taxonomy_term"
    }
  ]
}

Workbench could provide an option to include "revision_log" in creating or updating object.

@mjordan mjordan added the enhancement New feature or request label Apr 12, 2020
@mjordan
Copy link
Owner Author

mjordan commented Apr 12, 2020

The above works, but on update via REST, no new revision is created (the changed node replaces the current revision), even if the content type has "Create a new revision" set as the default in it publishing options. See https://drupal.stackexchange.com/questions/214301/how-do-i-force-nodes-to-have-revisions-by-default-when-using-rest for more info.

@mjordan
Copy link
Owner Author

mjordan commented Apr 12, 2020

This works:

<?php

/**
 * @file
 * Contains the islandora_workbench_integration.module.
 */

use Drupal\Core\Entity\EntityInterface;

/**
 * @param \Drupal\Core\Entity\EntityInterface $node
 *
 */
function islandora_workbench_integration_node_presave(EntityInterface $node) {
  if (preg_match('/revision/', $node->getTitle())) {
    $node->setNewRevision(TRUE);
    $node->setRevisionLogMessage('OK, title mentions "revision".');
  }
}

So we could hack in creation of revisions based on 1) that Islandora Workbench Integation module is enabled on the target Islandora and 2) the presence of a specific field on the incoming node to trigger the creation of a revision, for example 'workbench_revision_log_message' or similar that would only be present in nodes created by Workbench.

@mjordan mjordan added the limitation of dependency A known limitation resulting from another library/application label Feb 3, 2021
@Natkeeran
Copy link

@mjordan The approach you have outlined above seems good. We are interested in supporting revisions for media as well, and assuming a similar hook for media would work as well. Are there other considerations that we need to take into consideration before rolling out this work?

@mjordan
Copy link
Owner Author

mjordan commented Jul 6, 2023

We'd need to agree on how to represent the log messages in the input CSV. Might bet a bit complicated since you can create nodes, media, and taxo terms all in the same CSV row. If the same message applied to all entities, I guess you could introduce some config settings that would let you define the single message, e.g.

revision_messages:
 - node: Created by Islandora Workbench foo
 - media: Created by Islandora Workbench bar
 - term: Created by baz

or as CSV headers, node_revision_message, media_revision_message, and term_creation_message (all optional)

@mjordan
Copy link
Owner Author

mjordan commented Jul 6, 2023

Or we could use a field on the incoming entity as I described earlier. I guess it depends on whether the data that you'd put in the node field was worth keeping in the node, or the revision log message was purely transactional. If transactional, I think the CSV/config settings approach would be better.

@Natkeeran
Copy link

Providing a way to customize the update log message seems is valuable. The CSV headers approach (ex node_revision_message, media_revision_message, and term_creation_message etc) seems like a good approach. Wondering if we can detect the presence of this field in the pre_save. If so, then we can make creating revisions an option based on the existing of these field(s).

@mjordan
Copy link
Owner Author

mjordan commented Jul 6, 2023

Wondering if we can detect the presence of this field in the pre_save.

I can't imagine how, since we only have access to $node. Maybe in a custom HTTP header for entity creation requests, that the Integration module can grab?

@Natkeeran
Copy link

Natkeeran commented Jul 6, 2023

We can check if the Create new revision value set to True or False in the pre save. If True, create new revision, if false, don't create revision.

We don't need to setRevisionLogMessage, since it is already part of the json POST request body.

Not sure about preg_match('/revision/', $node->getTitle()) check in the above thread code.

@mjordan
Copy link
Owner Author

mjordan commented Jul 6, 2023

Sounds good.

Not sure about preg_match('/revision/', $node->getTitle()) check in the above thread code.

That was probably just some sample logic I used while figuring out how to not trigger a revision every time.

@mjordan
Copy link
Owner Author

mjordan commented Jul 7, 2023

I just thought of a complication - if we apply the revision message to media, do we apply it only to the media created from the file named in the file column? What about files created by additional_files?

@Natkeeran
Copy link

revision info would be relevant for the update process. In that case, each media would be in a separate row, and can have a specific message.

@hassanelsheikha
Copy link
Contributor

Hey @mjordan. I'm currently working on a solution for this issue (I hope this is OK with you). I've tested my code so far with media and nodes, and all works great. I vouch for @Natkeeran's approach in the previous comment about having users give the revision log message as a field in the CSV, and if they don't, then a default message will be written instead. I've implemented it like this, but if you'd like it changed then I would be happy to do whatever you see is best.

I also had a question about when to create revisions. After some investigation, the default value for the "Create new revision" checkbox is at the content type level, not the individual node/media levels. As such, I see one of two viable approaches:

  • Always create revisions
  • Acknowledge the default value for "Create new revisions" at the content type level and create revisions (or don't create them) according to that

I don't think the option to have the user specify if they want revisions for specific nodes is feasible, as the presave hooks do not know about the input CSV data.

I would appreciate your advise on this. Thank you so much!

@mjordan
Copy link
Owner Author

mjordan commented Jul 7, 2023

One question - are we considering applying a revision log entry during create tasks in addition to update_* tasks?

@hassanelsheikha
Copy link
Contributor

That shouldn't be much different than the implementation for update tasks, correct? Either way I can definitely implement this if you give the green light.

@hassanelsheikha
Copy link
Contributor

Actually, looking back now I think it makes most sense to only implement this for update tasks.

@mjordan
Copy link
Owner Author

mjordan commented Jul 10, 2023

Can you provide me with some sample CSV input data that contains revision messages for both nodes and media? I'm not entirely clear on how that will look.

@hassanelsheikha
Copy link
Contributor

Oh sorry. It seems like I may have misstated. What I meant is to simply give the user the ability to add their own revision message instead of use a default message (if they wish). But they will only be able to update either nodes or media at once and not both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request limitation of dependency A known limitation resulting from another library/application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants