Account created on 20 August 2017, over 8 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States j-barnes

@johan_den_hollander - This is excellent work! I tested the latest changes locally and everything is working great.

We're going to run this through our internal QA team on our staging environments to make sure everything holds up. Once they give the green light, I'll get this merged in.

Really appreciate all the work you put into this!

🇺🇸United States j-barnes

That would be great if you guys could look into this / take this up. I’ve been pretty swamped lately, so that would be amazing.

Thanks again for offering to help, I really appreciate it.

🇺🇸United States j-barnes

Thanks for the feedback on this! Just took a look at the code - both Amount and Age plugins should filter for status = 1:

  $buckets = $this->getRevisionIds($entity, [
    ['field' => $this->getStatusField($entity), 'value' => 1],
  ]);

So unpublished drafts (forward revisions with status = 0) should already be excluded from deletion. Are you seeing different behavior?

If you need more control, we could add a workflow-aware plugin that dynamically pulls in available moderation states and lets you pick which ones to clean up (draft, archived, etc.). Would that work better for your use case?

🇺🇸United States j-barnes
🇺🇸United States j-barnes

@johan_den_hollander - Awesome work! The provider architecture looks great and solves the fillable PDF issue.

One thing I noticed: phpexiftool/exiftool provides the binary (usually at vendor/phpexiftool/exiftool/exiftool), but we need a way to make it reliably accessible for pdf_metadata.

Thinking about a few approaches:

  • Binary auto-detection - check common locations (module vendor, project vendor, system PATH) with an optional admin override
  • Composer bin-dir - use post-install scripts to copy/symlink to an executable location
  • Manual path only - just provide the admin setting and leave setup to the user



The challenge is that vendor folders might not be executable or could be read-only in production environments (like Acquia). Not sure what the best approach is - what do you think?

Really appreciate your work on this - our team will test once we nail down the approach!

🇺🇸United States j-barnes
🇺🇸United States j-barnes

@anybody - I think it makes sense to implement this feature in revision_delete_tools as you suggested. I went ahead and created a new ticket to get a little more feedback on it here: https://www.drupal.org/project/revision_delete_tools/issues/3555771 Add bulk select & delete functionality to entity revision tab Active

🇺🇸United States j-barnes

j-barnes created an issue.

🇺🇸United States j-barnes
🇺🇸United States j-barnes

@vasike - Makes sense to me, we have a good bit of test coverage now too. Tagging a beta.

🇺🇸United States j-barnes

j-barnes made their first commit to this issue’s fork.

🇺🇸United States j-barnes

@anybody - Thanks for the suggestion! I know there's a core issue about this that's been around for a while. I'm wondering if it might make sense to pull some of the lighter parts from revision_delete_tools and contribute them there? Revision Delete Tools was originally created to just be a lightweight drush tool to quickly remove revisions on an as-needed basis.

That said, I'm also open to adding something to the revision tab here if you think that makes more sense. It shouldn't be too hard to tap into the existing system. Not sure which direction makes more sense though - curious to hear your thoughts. Either way, I'll make sure to add cross-reference information about both modules on the project pages so people know what's available.

🇺🇸United States j-barnes

Thanks for the patch! This fixed our issue where our orchestration agent was failing on subsequent messages after the first exchange. Our sub-agents are configured with return_directly: 1, and without this patch they weren't properly marking execution as finished, causing repeated responses. Working perfectly now!

🇺🇸United States j-barnes

We also love to have these merged for D11, thanks!

🇺🇸United States j-barnes

Hey @robertoperuzzo,

We ended up moving forward with the Solr extractor since it fit our workflow a bit better. I did make a few local changes that helped with timeouts, but they were more of a short-term workaround (increasing timeouts and such), so I didn’t think they’d add much value upstream. Really appreciate the work you’ve done here though.

🇺🇸United States j-barnes

j-barnes created an issue.

🇺🇸United States j-barnes

Closing this one out as it seems to be the same issue as https://www.drupal.org/project/pdf_metadata/issues/3480679 🐛 Fillable PDF fields are no longer fillable after saving Active . I'll update the other issue with this detail.

🇺🇸United States j-barnes

Added exception handling to QueueGenerator::enqueueIfUnique() to gracefully handle when multiple processes attempt to queue the same entity for revision cleanup simultaneously.

🇺🇸United States j-barnes

@sunny-lee - If you could take a look, this should now preserve characters now.

🇺🇸United States j-barnes

Nice catch, it looks like we are stripping out way too many characters when we don't need to. Instead of trying to remove the "unsafe" characters I think it's best to just escape the few that PostScript actually cares about. I'll add a merge request to fix this.

🇺🇸United States j-barnes

Merged! Thanks for all of the work on this!

🇺🇸United States j-barnes

@robertoperuzzo – thanks for the work on this!
Our team was looking for a way to leverage search_api_attachments with Unstructured to clean up the text for our RAG search, and this is the perfect solution. We have tons of legacy content that hasn’t been OCR’d, so this works well for that.

I did run into a few issues and have a couple of wish-list items:

  • Chunking elements – the form values don’t persist after save (adding those fields to submitConfigurationForm() fixed it for us).
  • Chunking settings not applied – the options aren’t being included in the payload request, so they don’t appear to take effect yet.
  • Large files time-out – anything over ~1 MB (we have one that’s 1.1 MB) hits a DelayedRequeueException loop. Increasing the Guzzle time-outs solved it locally; exposing these as configurable options would be great.
php
  $options = [
    RequestOptions::HEADERS => [
      'Accept'               => 'application/json',
      'unstructured-api-key' => $api_key?->getKeyValue() ?? '',
    ],
    RequestOptions::MULTIPART => [
      [
        'name'     => 'files',
        'contents' => $file_resource,
        'filename' => $file->getFilename(),
        'headers'  => [
          'Content-Type' => $file_mime_type,
        ],
      ],
      [
        'name'     => 'strategy',
        'contents' => 'ocr_only',
      ],
    ],
    RequestOptions::TIMEOUT         => 300,
    RequestOptions::CONNECT_TIMEOUT => 30,
    RequestOptions::READ_TIMEOUT    => 300,
  ];
  • Expose strategy choices – it would be awesome to have a dropdown for extraction strategy (e.g., ocr_only, high_res, etc.) so we can pick the best option for non-OCR’d PDFs.



Anyways, great work on this -- hope we can get this merged in soon. Attaching PDF that we have been having issues with if you need it for testing.

🇺🇸United States j-barnes

Enabled Gitlab CI and added some basic functional tests to validate the functionality.

🇺🇸United States j-barnes

@pmagunia - thanks for reporting this. I've added a new constraint that should now be used on 10.2+ for validation. Going to try and write up some tests for this before getting it in.

🇺🇸United States j-barnes

We are currently experiencing the same issue on 10.4.7 with ai / ai_agent on latest dev release. It looks like it's caused by the node_unpublish_by_keyword_action and returned no attributes. Adding the above seems to have cured our problem.

🇺🇸United States j-barnes

@raveen_thakur51 - Thanks for adding that, I've went ahead and fixed the phpcs and cspell issues as well. We should be good to go now.

🇺🇸United States j-barnes

@mkalkbrenner - this is working much better now! Do you think it's worth adding some extra logic in the getRevisionableEntityTypes() to filter out some of the entity types that might not be applicable like paragraphs or unique ones like menu_link_content where the entity_type_id might differ?

🇺🇸United States j-barnes

Thanks for the work on this, this is looking great. Going to do some testing on it and we will get it in.

🇺🇸United States j-barnes

Good catch and thank you!

The correct command for should:
``drush rm:queue`` - Queue all enabled entities for revision deletion processing.

I've updated that part of the form title as it doesn't really make sense to have it under the "disable_automatic_queueing" section.

🇺🇸United States j-barnes

We’re experiencing this same issue with larger file uploads. When using the widget, files are uploaded directly to S3 client-side. However, the fileInput element still retains the selected file. This means when the form is later submitted via FormData, the file is included again in the request body — even though it was already uploaded — which can cause server-side errors such as exceeding post_max_size.

I think the simple solution is just to clear the fileInput value so that FormData doesn't include the file.

🇺🇸United States j-barnes

j-barnes made their first commit to this issue’s fork.

🇺🇸United States j-barnes

It looks like the project is listed under community projects which is has some limitations.

In the meantime, I’ve been using the patch below to purge the items once it reaches a specified size:

https://www.drupal.org/project/admin_audit_trail/issues/3197592 Add settings to toggle between expanded/collapsed filters. And set an option to limit database table size Needs work

🇺🇸United States j-barnes

Updated the latest patch to include a small change to allow for patterns similar to how config ignore works. We encountered a use case where enabling client-side validation for all webform submissions was necessary. However, manually adding hundreds of form IDs was impractical.

Example:

webform_submission_name_change_test_form
webform_submission_*
🇺🇸United States j-barnes

@nicholass – Thanks for the patch! It's working great. I've rebased this and made a few minor updates, including dependency injection and some coding standard improvements. I'm also opening a merge request to get more eyes on it. Appreciate your work on this!

🇺🇸United States j-barnes

Also looking at including this in our project if a release is created, thanks!

🇺🇸United States j-barnes

Awesome, our team was actually just asking about this functionality. Seems to be working great. Thanks for the contrib!

🇺🇸United States j-barnes

I've added a new token access test based off the webform populate test. Let me know if you'd like any changes or if you'd prefer some kind of base class added. This should at least get us headed in the right direction.

🇺🇸United States j-barnes

@Joe - Thanks for the contribution, this is working great for our team. Created the MR for this so we can hopefully get it merged soon.

🇺🇸United States j-barnes

Seems to be related to and fixed in: https://www.drupal.org/project/drupal/issues/2896169 🐛 Details elements have incorrect aria-describedby attributes Needs work

🇺🇸United States j-barnes

That makes sense to me, I'll do that going forward. I fixed the one issue, but I think the others may be unrelated. See below:

------ -------------------------------------------------------- 
Line   tests/src/Functional/HandlePdfControllerTest.php        
------ -------------------------------------------------------- 
20     @coversDefaultClass references an invalid class         
       \Drupal\fillpdf\Controller\HandlePdfController          
                                                               
       Also covers \Drupal\fillpdf\Plugin\FillPdfActionPlugin  
       and \Drupal\fillpdf\OutputHandler..                     
       🪪  phpunit.coversClass                                 
------ -------------------------------------------------------- 
[ERROR] Found 1 error 

FILE: ...3/web/modules/custom/fillpdf-3460893/tests/src/Traits/TestFillPdfTrait.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 112 | WARNING | [x] Empty PHP statement detected: superfluous semicolon.
     |         |     (Generic.CodeAnalysis.EmptyPHPStatement.SemicolonWithoutCodeDetected)
--------------------------------------------------------------------------------
🇺🇸United States j-barnes

Thanks for the feature, this is working great for us! I went ahead and updated / refactored this for the new 5.2.x-dev branch.

🇺🇸United States j-barnes

j-barnes changed the visibility of the branch 3460893-webform-fillpdf to hidden.

🇺🇸United States j-barnes

We are currently experiencing the same issue on the latest Webform 6.3.x-dev utilizing the name element when dealing with multiple values.

Below is the outputted value using a dump on the submission view, so the input does exist.

After digging a bit, you can see the below value that gets passed for rendering. The template item-list expects to receive a flat structure, so the item is never rendered. The table view uses a different composite rendering function, is unaffected by this issue.

A simple fix to get us started is to add modifications to: docroot/modules/contrib/webform/src/Plugin/WebformElementBase.php

if ($item) {
    $items[] = is_array($item) ? reset($item) : $item;
}

This adjustment flattens the item, allowing it to render correctly. However, further review is needed to ensure there are no unintended side effects from this change.

🇺🇸United States j-barnes

Attaching patch with update hook that we used to fix the issue.

🇺🇸United States j-barnes

Thanks for the patch, working great for us. Re-rolling against the latest changes.

🇺🇸United States j-barnes

j-barnes made their first commit to this issue’s fork.

Production build 0.71.5 2024