Account created on 30 July 2007, about 18 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States majorrobot

Pretty straightforward!

🇺🇸United States majorrobot

majorrobot made their first commit to this issue’s fork.

🇺🇸United States majorrobot

majorrobot created an issue.

🇺🇸United States majorrobot

Thanks for finding that, @dmundra. That didn't occur in my testing at first, but now I'm seeing it.

It looks like we need to increase the maxOutputTokens value when using Gemini. It uses reasoning and thus “thinking tokens” which apparently count against your output. I increased the maxOutputTokens by a factor of 4 (8096->32384), and it ran without issue.

Updated the QA steps.

🇺🇸United States majorrobot

This is ready for review.

QA Steps
Note: you will need an Anthropic key and a Gemini key to test this branch.

  • Roll back any migrated content: ddev drush migrate:rollback simple_content_migration
  • Clear cache: ddev drush cr
  • Import with Anthropic (which is pre-set in the example module's yml): ddev drush migrate:import simple_content_migration
  • Confirm 2 nodes import with no issues.
  • If you have access, go to your Anthropic API account logs and confirm that Drupal queried Anthropic: https://console.anthropic.com/workspaces/default/logs
  • Roll back: ddev drush migrate:rollback simple_content_migration
  • Update modules/ai_migration_example/migrations/simple_content_migration.yml, change lines 13-19 to:
          provider_id: gemini
          model_id: gemini-2.5-flash
          config:
             temperature: 0
             topP: 0
             topK: 0
             maxOutputTokens: 8096
    
  • Clear cache: ddev drush cr
  • Run the import from Gemini: ddev drush migrate:import simple_content_migration
  • Confirm 2 nodes import with no issues.
  • Go to your Gemini API usage page, choose your project, and confirm that you queried Gemini. (Note, the reports may not use your local time.)
🇺🇸United States majorrobot

Removed caveat around using Gemini.

🇺🇸United States majorrobot

Thanks @dmundra! Merged.

FYI, it turns out this was addressed in AI core: https://www.drupal.org/project/ai/issues/3479159 🐛 cURL Timeout and Deprecated Function Error When Using AI Translate Submodule Active . It looks like you can add the config when you create the provider instance.

 $provider = $this->aiProviderPluginManager->createInstance($this->providerId, [
      'model_id' => $this->modelId,
      'http_client_options' => [
        'timeout' => 300,
      ],
    ]);

For posterity:

QA Steps:
This assumes you have already added a Gemini API key to your Drupal instance.

  • Log in as an admin.
  • Go to /admin/config/ai/settings.
  • Under Chat, set the Default Provider to Gemini and the Default Model to Gemini 2.5 Flash.
  • Rollback any previous imports: ddev drush migrate:rollback simple_content_migration
  • ddev drush cr
  • Import: ddev drush migrate:import simple_content_migration
  • Confirm that 2 items imported and you do not receive a curl error.
🇺🇸United States majorrobot

Merged. Thank you, @dmundra!

🇺🇸United States majorrobot

majorrobot created an issue.

🇺🇸United States majorrobot

Thank you @timozura and @dmundra!

🇺🇸United States majorrobot

majorrobot made their first commit to this issue’s fork.

🇺🇸United States majorrobot

Removed HTML sanitization, which is complete, and added item for model configuration.

🇺🇸United States majorrobot

Added Tested Models section.

🇺🇸United States majorrobot

This one is ready for review.

FYI, I added OPT_IN_TEST_PREVIOUS_MAJOR: 1 in our .gitlab-ci file so we can test against D10. Unfortunately, this led to an older version of PHPStan testing also. I attempted to resolve this by adding a phpstan.neon, but looks like I broke all the PHPStan testing. Will need to figure that out.

Ideally we would test in D10 and D11. Testing in at least one would be good.

QA Steps

  1. ddev drush cr
  2. Rollback any migrations you've done: ddev drush migrate:rollback simple_content_migration
  3. Test a migration. It seems like OpenAI 4.1 is working pretty well today. ddev drush migrate:import simple_content_migration
  4. Note that you may experience some curl timeout issues; this has nothing to do with the code changes.
  5. Verify that, barring curl issues, 2 pieces of content migrated successfully.
🇺🇸United States majorrobot

majorrobot created an issue.

🇺🇸United States majorrobot

majorrobot changed the visibility of the branch 3546717-modify-maxtokens-for to hidden.

🇺🇸United States majorrobot

Thanks for your work on this @webbywe! This will save us some tokens. :)

This is merged in.

🇺🇸United States majorrobot

I've tested the fork, and it worked as intended (and helped us get past a painful token bottleneck). It would be great to have this feature available. Thanks for your work on this!

(I suspect it may need a rebase, tho.)

🇺🇸United States majorrobot

majorrobot created an issue.

🇺🇸United States majorrobot

Thanks for testing @dmundra. I'll take a look, too.

I assume you were using stable versions for Open AI and Anthropic provider modules? I wonder if it would make any difference to use dev versions.

🇺🇸United States majorrobot

majorrobot created an issue.

🇺🇸United States majorrobot

Adding information for the module road map

🇺🇸United States majorrobot

Adding documentation around the module's constraints.

🇺🇸United States majorrobot

Good catch @dmundra. I've added in the missing config and moved everything to a config/optional directory so that we can uninstall/enable the module without a configuration objects already exist in active configuration error (something that was occurring to me while testing this morning).

🇺🇸United States majorrobot

I've made the example content more general. I used posts from accessibility.civicactions.com for the slice.

I've also removed the old example content type and started over -- the new content type is simple_content_migration. I removed the old migration yaml and cleaned up the example yaml.

QA Steps:

  1. Disable the example module. ddev drush pmu ai_migration_example
  2. Re-enable the module to create the new content type. ddev drush en -y ai_migration_example
  3. Log in and go to admin/structure/types.
  4. Confirm that simple_content_migration is in the list.
  5. It is advisable to delete the old content type easy_migration_example.
  6. Clear the cache. ddev drush cr
  7. Run the import. ddev drush migrate:import simple_content_migration
  8. Confirm that 2 items import.
  9. Go to the content list and inspect the nodes.
  10. Confirm that they imported adequately.
🇺🇸United States majorrobot

@danielmundra potential upcoming change in StructuredOutput API:

https://www.drupal.org/project/ai/issues/3545462 📌 Move the input of structured output to an object. Active

🇺🇸United States majorrobot

This is ready for review. Here are some QA steps:

This assumes that you already have the module set up in a local sandbox.

  1. Check CONTRIBUTING.md and make sure you have enabled all of the modules listed in #10. Especially note serialization has been added.
  2. Clear cache. ddev drush cr
  3. Attempt to run a sample migration: ddev drush migrate:import slice1_content_with_passthrough
  4. Confirm there are no errors with the migration
  5. If you want to re-run the migration, roll it back first: ddev drush migrate:rollback slice1_content_with_passthrough
  6. After your first run, the AI response is cached. Migrations that are run after that will work with the same AI response. Clear the cache if you would like the migration process to ping the AI provider again.
🇺🇸United States majorrobot

New MR for review: https://git.drupalcode.org/project/ai_migration/-/merge_requests/16

This one switches out the hard-coded bundle schemas with a dynamic method that uses the Schemata module and core serialization. The result may be a bit overkill, but is progress — and if nothing else — proof of concept.

To test:
1. ddev poser
2. Follow CONTRIBUTIONS.md for the modules to enable. In most cases, you will need to:
3. ddev drush en schemata,schemata_json_schema,jsonapi
4. ddev drush cr
5. Run ddev drush ev "\Drupal::service('ai_migration.ai_migrator')->convert('https://lincs.ed.gov/professional-development/resource-collections/profi...', '', 'node', 'easy_migration_example');"
6. Log in as an admin.
7. Go to /admin/reports/dblog
8. Locate a record that begins with “AI migration schema for url…”
9. Click on the record.
10. Verify that it there is a json schema in the message. (Apologies -- you may want to copy the json out of the message and paste into a tool that will prettify it.)

🇺🇸United States majorrobot

Thanks for this feature, @webbywe!

I reviewed the MR and tested locally. Everything worked locally.

I left a few comments and questions on the MR -- nothing big. Thank you again!

🇺🇸United States majorrobot

I've played with the Schemata module, and can get a bundle schema created using that and jsonapi.

I think we need to merge in https://www.drupal.org/project/ai_migration/issues/3539641 📌 Support images in AiMigrator Active first, as there are a lot of changes in there that this issue could use.

🇺🇸United States majorrobot

majorrobot changed the visibility of the branch 3538092-spike-investigate-schema to hidden.

🇺🇸United States majorrobot

Noting here that I merged in @webbywe's branch into this issue's fork. Thank you for your contribution on stubs, webbywe! That looks like a more logical way to go, much less confusing than starting migration at the bottom (files) and working up through media, to content.

I think we still need to figure out how to get the migration to create the image urls from the stubs correctly, so I'm leaving this one open.

We also have some failing tests to fix.

🇺🇸United States majorrobot

Also, I know PHPUnit tests are failing ... I didn't get a chance to update those since the last MR. If anyone wants to take that on, have at!

🇺🇸United States majorrobot

I've almost got the module migrating an image into Files. It was working earlier today, but I somehow broke it -- I think I've done something to slice1_image_files and the file_copy plugin is not getting a parameter it needs. See attached screenshot.

Nevertheless, it would be great to get a review of what's there thus far. Also, if someone else wants to pick it up and add commits, please do!

🇺🇸United States majorrobot

Sry dmundra. I should start putting QA steps in these things.

There’s nothing really to test beyond the migration not being broken. Thx!

🇺🇸United States majorrobot

I'm thinking we might not want to merge this atm -- we will have the update the commit ref in `composer.json` whenever there is a new commit in the MR. I'm thinking we should wait to act until the change is merged in or at least more stable. Thoughts, @dmundra?

🇺🇸United States majorrobot

Thanks for doing this, @dmundra! What is your thought re: getting Structured Output added? Does this need to be merged before that can be done?

🇺🇸United States majorrobot

Thx to everyone who mobbed and helped us resolve issues.

@dmundra, I've committed all of the suggestions. This is ready to go again.

🇺🇸United States majorrobot

Sry about that @dmundra. There was a extraneous field storage config I had added to address a test that I later removed. I think I've resolved the issue now.

Yes, a good test command is ddev drush mim slice1.

Thank you!

🇺🇸United States majorrobot

MR is ready for review.

  • Adds an Ai data_parser plugin that utilizes our AiMigrator service.
  • Adds a test migration yml for an example slice of urls.
  • Adds an additional config file for our sample content type. It is necessary for loading the content type during tests.
  • Updates AiMigrator so that convert() takes url, html, or both. Improves Ai prompt.
  • Updates the hard-coded bundle structure for AiMigrator so that it resembles an actual Drupal bundle and can be migrated easily.
  • Updates tests.
🇺🇸United States majorrobot

I've created a submodule called ai_migration_example. On installation it creates a content type called Easy Migration Example with fields that match what we're already pulling in our AiMigrator service:

   $bundle = (object) [
      'title' => '',
      'introduction' => '',
      'resource URL' => [
        'text' => '',
        'url' => '',
      ],
      'authors' => [],
      'abstract' => '',
      'related topics' => [
        [
          'text' => '',
          'url' => '',
        ],
      ],
    ];

Adding a sample migration yml file and a custom data parser plugin will be in a future issue.

🇺🇸United States majorrobot

I've pushed up a branch with some work into using \Drupal::service('entity_field.manager')->getFieldDefinitions('node', ‘[bundle]'). I don't necessarily think it's going to work out, but I wanted to push it up for reference.

https://git.drupalcode.org/issue/ai_migration-3538092/-/tree/3538092-spi...

🇺🇸United States majorrobot

I’ve hardcoded a PHP object that represents a bundle from the Department of Education's LINCS website, as an example.

To test locally, try:

ddev drush php-eval "print_r(\Drupal::service('ai_migration.ai_migrator')->convert('https://lincs.ed.gov/professional-development/resource-collections/prle-8850'));"

Other pages you could try:

🇺🇸United States majorrobot

This MR is ready. We already had a basic MVP prompt builder, but I've added a couple of things:

* Updates the system role passed to the Provider, with You are an API that attempts to parse HTML content and find the important parts of a web page.
* Since we can't do Structured JSON calls with Gemini yet, I passed the schema with the system role instruction You will return this as a JSON object with this schema: ' . $schema)
* Given a PHP object (like a bundle, say), the createSchema() method can now generate jsonSchema on the fly. This is thanks to evaisse/php-json-schema-generator. This feels like a big win.

With these updates, Gemini can look at html and return an object along the requested schema with no specific instructions about the page. That is exciting and pretty awesome.

I've gotta credit https://www.raymondcamden.com/2024/11/27/using-generative-ai-to-parse-we... for helping me understand how to work with Gemini.

🇺🇸United States majorrobot

Thx @dmundra. Merged and marking as Fixed.

🇺🇸United States majorrobot

Please note that I am aware of the PHPStan issues -- I did not use DI with the logger service in order to quickly get logging in. I can refactor this to DI later.

🇺🇸United States majorrobot

This is ready for review. Here's an easy way to QA:

ddev drush php-eval "print_r(\Drupal::service('ai_migration.ai_migrator')->convert('https://example.com'));"

Replace example.com with any url.

🇺🇸United States majorrobot

Sry, somehow assigned this to the wrong person.

🇺🇸United States majorrobot

Thanks for your review @dmundra. I've merged it in. Onward and upward!

🇺🇸United States majorrobot

[Removing LINCS as customer from my attributions.]

🇺🇸United States majorrobot

MR is created. Basic skeleton of the migrater service is there. I had AI write some Kernel Tests. Tests of the `convert()` method got too complicated and time-consuming to correct/update, so I pulled them for now.

... and I just realized that technically it should be "migrator" and not "migrater." Doh.

🇺🇸United States majorrobot

Fixed the phpcs, and added a couple of useful bits in .gitignore and Contributing.md.

🇺🇸United States majorrobot

Added in the MR!

🇺🇸United States majorrobot

^^ I rerolled again so the patch will apply. But I noticed, we're failing 3 PHPUnit tests now.

I think we'll also need to update js/gtm.js so that there can be > 1 dataLayer name. Right now, everything is going into dataLayer.

So, more work to be done, but patch works again.

🇺🇸United States majorrobot

Updating version since this seems to still be an issue in the latest module version.

🇺🇸United States majorrobot

My team also ran into this issue. The patch worked perfectly for us.

We also ran a node_update_N() to clear nodes from the index that we had already deleted (we only had a handful of nodes). This might be helpful for others:

function my_module_update_N(&$sandbox) {
  $nids = [...]; // Your list of nodes.

  $search_index = \Drupal::service('search.index');

  foreach ($nids as $nid) {
    $search_index->clear('search_exclude_node_search', $nid);
  }
}
🇺🇸United States majorrobot

Removed old USWDS v1 (or v2?) CSS. The newer versions of the crumbs look good without it.

🇺🇸United States majorrobot

@kentr -- agreed. Admin Toolbar uses the admin theme's toolbar template, and the root issue -- for me, at least -- was in the Claro theme.

🇺🇸United States majorrobot

This is likely caused by the template your admin theme is using.

I found that I was able to resolve these issues by overriding the toolbar template. It was a bit tricky since I had to add a template suggestion to wrest control out of my admin theme's (Claro's) grip.

I put this in my .theme file:

function my_theme_theme_suggestions_toolbar_alter(array &$suggestions, array $variables) {
  // Add theme hook suggestions for toolbar so that we can override Claro. Otherwise, we just get Claro's template.
  if (!empty($variables['element']['#attributes']['id'])) {
    $suggestions[] = 'toolbar__' . str_replace('-', '_', $variables['element']['#attributes']['id']);
  }
}

Then I copied Claro's toolbar.html.twig into my theme as toolbar--toolbar-administration.html.twig.

In the twig file, I saw


{% for key, tab in tabs %}
   ...
    <nav class="toolbar-lining clearfix" role="navigation">
   ...
{% endfor %}

Which was causing trouble -- there's not a unique name on this element, which could be used several times. So I added an iterator:

{% set i = 0 %}
{% for key, tab in tabs %}
   ...
    <nav class="toolbar-lining clearfix" role="navigation" aria-label="admin-toolbar-nav-{{ i }}">
   ...
{% set i = i + 1 %}
{% endfor %}

For your second issue (which I also had), I added a landmark to wrap the whole toolbar. I haven't run this past any accessibility experts yet, but this does pass Axe's test:

<section id="toolbar-wrapper">
  <div{{ attributes.addClass('toolbar') }}>
    <nav{{ toolbar_attributes.addClass('toolbar-bar', 'clearfix') }}>
   ...
</section>

I hope this helps someone!

🇺🇸United States majorrobot

#49 works for me. Passes Axe Core automated testing. I also tested with keyboard and screenreader, and it all worked.

I tested with version 2.2 of the module.

I didn't see the issue in #50 📌 Provide an accessible variant Needs review . Going to set this to Needs Review.

🇺🇸United States majorrobot

@emptyvoid -- curious what the difference between this issue and patch and https://www.drupal.org/project/ckeditor_accordion/issues/3124167 📌 Provide an accessible variant Needs review ?

🇺🇸United States majorrobot

This seems like a good feature to replace the Contact Form Permissions module (which appears abandoned and doesn't have a supported release anymore). So I'm excited to see it in progress!

I've created a fork/MR from @naveenvalecha's patch in #18 and updated core version requirements.

I've also attempted to address @larowlan's requests in #22.

+++ b/modules/contact_permissions/src/ContactPermissions.php
@@ -0,0 +1,55 @@
+class ContactPermissions {
This needs to use \Drupal\Core\Entity\BundlePermissionHandlerTrait now so that we get the correct dependencies

I've added this, as I understand it -- but this is my first time handling permissions in Drupal 8+, so I may have missed something.

+++ b/modules/contact_permissions/contact_permissions.module
@@ -0,0 +1,61 @@
+ $entity_types['contact_form']->setAccessClass('\Drupal\contact_permissions\ContactFormAccessControlHandler');
we also need to switch the permissions route provider from EntityPermissionsRouteProviderWithCheck to EntityPermissionsRouteProvide

I'm actually not following this one -- I don't see where EntityPermissionsRouteProviderWithCheck is used. Nor have I been able to figure out the intent of the note with a little research. Happy to make changes if someone could clarify, though.

🇺🇸United States majorrobot

majorrobot made their first commit to this issue’s fork.

🇺🇸United States majorrobot

#14 worked for my case. We have a multi-value field that allows a maximum of 2 values. Previously, you could add as many values as you want -- the values would not save, nor would an error be thrown.

After applying a patch from #14, user gets an error when >2 values are chosen.

🇺🇸United States majorrobot

Can confirm this is also broken in 4.0.2.

However, the most recent fix (3309324-single-day) does the trick (at least for the dev version of the module). Would be great to get this merged in and released so we don't have to depend on the dev version!

🇺🇸United States majorrobot

Hey @jminarick, this makes sense. Better to let the theme / individual site owner decide how to deal with that header. Looks good.

🇺🇸United States majorrobot

I can verify this works. I removed the title setting and it kindly no longer appeared.

Production build 0.71.5 2024