teknorah β credited ultimike β .
ultimike β created an issue. See original summary β .
So is the plan that both drupal_cms_ai
and drupal_cms_analytics
will be apply-able in Drupal CMS 1.0 only via the command line? Or is that TBD?
-mike
drupal_cms_search
is a weird one because it appears in the drupal_cms_starter
recipe's composer.json file but not in the recipe.yml file - maybe this is an oversight?
I agree that drupal_cms_ai
, drupal_cms_analytics
, and drupal_cms_multilingual
should all be available in Project Browser.
-mike
Does #8 work with the latest version of this module?
The current version of DDEV includes npm 8.19.4 - it would be great if this module supported this version of npm out-of-the-box, and it would make contributing to this module **much** easier.
Or - better yet - close this issue in lieu of π Switch from deprecated Node Sass to Dart Sass compiler and change minimum node version to 16.0 and later in the Bootstrap Styles module Active
-mike
This MR worked for me as well with Drupal core 10.3.9, Group module 3.3.0, and version 2.1.1 of this module.
-mike
johnpicozzi β credited ultimike β .
One other thought on this - as things currently stand, Project Browser will only work with recipes that are installed with Composer. If you download/create a recipe into a project, Project Browser won't find it (unless you've previously used Composer to require an additional recipe to the same directory.)
Regarding
When I do a composer require somevendor/somerecipe (where the project's composer.json has "type": "drupal-recipe", where is the Composer configuration that installs this dependency in the project root's "recipes" directory? (using a Drupal 11.0.7 site based on the drupal/recommended-project Composer template.) This is a bit of a mystery.
Credit to @ctrladel for figuring out that recent versions of composer/installers has this gem:
https://github.com/composer/installers/blob/main/src/Composer/Installers/DrupalInstaller.php
Mystery solved!
-mike
Doh! Never mind - I think the plugin already does this!
In Drupal 10 and and 11, it searches the project-root's "recipes/" directory.
From a code comment:
// The recipe system requires that all non-core recipes be located next to each
// other, in the same directory.
So, no "custom" or "contrib" directories allowed.
When I do a composer require somevendor/somerecipe
(where the project's composer.json has "type": "drupal-recipe", where is the Composer configuration that installs this dependency in the project root's "recipes" directory? (using a Drupal 11.0.7 site based on the drupal/recommended-project Composer template.) This is a bit of a mystery.
Sidenote: I have noticed that the 11.x drupal/recommended-project Composer template includes "recipes/{$name}": ["type:drupal-recipe"]
.
Finally, in the Project Browser recipes plugin, there is this bit of code that uses information from Composer to figure out which directories Project Browser searches for recipes:
// If any recipes have been installed by Composer, also search there. The
// recipe system requires that all non-core recipes be located next to each
// other, in the same directory.
$contrib_recipe_names = InstalledVersions::getInstalledPackagesByType(Recipe::COMPOSER_PROJECT_TYPE);
if ($contrib_recipe_names) {
$path = InstalledVersions::getInstallPath($contrib_recipe_names[0]);
$path = $this->fileSystem->realpath($path);
$search_in[] = dirname($path);
}
Bottom line - nothing to see here, closing this issue, leaving this information for those that are curious.
-mike
After discussions with one of the Recipe leads, it was determined that we should be including both /project-root/recipes
and /project-root/web/recipes
.
I've updated the issue summary.
-mike
ultimike β created an issue.
johnpicozzi β credited ultimike β .
I completely agree with @damienmckenna, @herved, @loopy1492, and others - removing update hooks is a bad idea.
-mike
lostcarpark β credited ultimike β .
I think I'm in a similar situation as the OP (@fessouma) where I can't figure out what I'm missing. Here's some details:
- I just pulled the latest -dev.
- I have "Stream" unchecked in the "AI Chatbot" block configuration.
- I can put my chatbox prompt in the "AI Vector DB Explorer" and get 3 results that are above my context threshold (0.1).
- I am only getting the generic answers from the chatbot ("I'm sorry, I can not find a database to look this up in.")
- Looking at the logs, after I use the chatbot, I only see a call to OpenAI to the "gpt-4o" modul - I do not see a call to the "text-embedding-3-small" model which makes me think that chatbox prompt is not getting embeddings-ized in order for the RAG search of my local Milvus database to happen. I should always see an initial call to the "text-embedding-3-small" model for the initial prompt, right?
In my head (which may be incorrect), I'm thinking that the following things should happen:
- The chatbox prompt gets embedding-ized.
- The local Milvus database is searched using the embedding-ized chatbox prompt.
- The results of the previous step are added to the prompt instructions for the gpt-4o call.
So, when I look in the AI Logs, I am expecting to see the "Extra data" section contain the prompt instructions text along with results from the local RAG search. Am I not thinking about this the right way?
thanks,
-mike
Allowing subfolders could lead to naming collisions as you could have a content_type_page in multiple sub-folders and which one would a recipe that depends on it apply?
@thejimbirch - ah, makes sense. Thanks for the explanation.
-mike
Big +1 to the the 3 type options proposed (Foundational, Kit, and Site).
When I look at the project root for Drupal CMS, or within any module that contains multiple recipes, I think it would be helpful if there was a directory structure that mirrors these types. Off the top of my head:
/recipes/foundational/...
/recipes/kit/...
/recipes/site/..
This way, a developer would be able to quickly identify which was which without inspecting their recipe.yml file.
As for a second classification type, I don't have a strong opinion other than a standard naming convention could go a long way in providing this type of classification for foundational recipes.
-mike
@alfthecat - Thanks for the kind words!
Also - I loved your TV show when I was growing up!
When the setFile()
method is used to specify the location of the batch functions, it must be called before setFinishCallback()
or the finish callback will not be found:
$batch_builder = (new BatchBuilder())
->setTitle($this->t('Processing Batch'))
->setFile('/my_module.batch.inc')
->setFinishCallback('my_finish_callback_function')
->setInitMessage($this->t('Batch is starting'))
->setProgressMessage($this->t('Processed @current out of @total.'))
->setErrorMessage($this->t('Batch has encountered an error.'));
I think this is fine - if others feel a need for a DESC option, then a new issue can be opened. In my case, simpler is better!
thanks,
-mike
ultimike β created an issue.
I can confirm that in my case, a CSV file with 210,000+ rows and 17 columns loaded and performed well on my local with Drupal 10 and a standard DDEV configuration.
-mike
liberatr β credited ultimike β .
liberatr β credited ultimike β .
Merged!
ultimike β created an issue.
ultimike β created an issue.
ultimike β created an issue.
Tests are passing. Needs review.
-mike
@drubolbol and @nico059 did a great job during the DrupalCon Barcelona mentored core contribution day - making good progress on this issue (and learning a lot about the contribution process along the way.)
Currently, it appears that the changes have caused some tests to fail:
- Drupal\Tests\file\Functional\SaveUploadTest
- Drupal\Tests\file\Functional\SaveUploadFormTest
- Drupal\Tests\file\Functional\RemoteFileSaveUploadTest
See https://git.drupalcode.org/issue/drupal-3368857/-/pipelines/294400 for more details.
-mike
I will be working on this issue during the DrupalCon Barcelona mentored contribution with @pierregermain
After September 28, 2024, feel free to pick this issue up and continue.
-mike
Is the following correct for core lib tests?
Unit tests: core/lib/tests/Drupal/Tests/Core (\Drupal\Tests\Core\)
Kernel tests: core/lib/tests/Drupal/KernelTests/ (\Drupal\KernelTests\Core\)
Functional tests: core/lib/tests/Drupal/FunctionalTests/ (\Drupal\FunctionalTests\Core\)
Functional Javascript tests: core/lib/tests/Drupal/FunctionalJavascriptTests/ (\Drupal\FunctionalJavascriptTests\Core\)
-mike
I plan on working on this issue with my team.
-mike
ultimike β made their first commit to this issueβs fork.
ultimike β made their first commit to this issueβs fork.
Perfect!
thanks,
-mike
I think you may want to considering updating the help text, as it doesn't feel complete to me. The fact that Advanced mode is dependent on not only the Token module being available but also the Automator type probably warrants additional help text. Know what I mean?
-mike
@marcus_johansson - I didn't have anything specific in mind, I just thought it was odd that I couldn't access advanced mode even though I had token module installed.
I didn't realize that access to advanced mode was dependent on the automator type.
-mike
@lostcarpark - thanks for looking into this. As we discussed during DrupalEasy office hours, my ideal solution would be (in order of my preference):
league/commonmark magically introduced a new GitLabFlavoredMarkdownConverter
class that I could just drop-in and have it work.
Some other wonderful open-source minded person or organization created a PHP dependency for GitLab-flavored Markdown that I could include just about as easily as the first option above.
I don't want to get in the business of partially supporting GitLab-flavored Markdown by enabling Commonmark extensions - if we do that, I feel the chances are currently low that we'll ever be able to claim 100% compatibility with GitLab Markdown.
-mike
ultimike β created an issue.
ultimike β created an issue.
ultimike β created an issue.
@jacobupal,
No, tokens that include a max-length value are not supported.
When using the new Smart Trim token, the max value (and the rest of the settings) will come from the Token view mode of the entity (if specified) or fallback to the default view mode settings.
-mike
@marc.bau - exactly which commit are you talking about when you say,
but a commit has pushed to dev 2 years ago
thanks,
-mike
ultimike β created an issue.
Hopefully, this is now all set.
During DrupalEasy office hours today, we updated the text fixture - it was a great learning experience for all of us, and definitely a team effort. I have asked those to participated to leave a comment below so that they can receive credit as well.
π€πΌthat all tests pass.
Some notes along the way:
To import the old database fixture, we first deleted all the tables from the database, then used the following command to import (from the "web" directory): php ./core/scripts/db-tools.php import ./modules/contrib/smart_trim/tests/fixtures/update/drupal-10.0.8-smart_trim-2.0.php
Prior to running the database updates for Drupal core, we checked out the 2.0.0 branch of Smart Trim (otherwise we would have run the database updates that are meant to be tested). After we ran the updates, we checked out the -dev for Smart Trim.
To re-create the fixture, we used (again from the web directory): php ./core/scripts/db-tools.php dump-database-d8-mysql > modules/contrib/smart_trim/tests/fixtures/update/drupal-10.3.2-smart_trim-2.0.php
, then we used "gzip" to compress the file.
-mike
@ankitv18 - I _think_ I understand what you mean, but not entirely 100% sure.
In walking through all this during DrupalEasy office hours, we understand that it is not possible to upgrade from Drupal core 10.0.* to Drupal 11 (next major), which is why you're suggesting we update the "drupal-10.0.8-smart_trim-2.0.php.gz" test fixture to be based on Drupal core 10.3.x.
But, Drupal 11 has been the "next major" for quite some time now, so I'm not sure I understand why this is suddenly an issue.
For example, on April 22, 2024, it was failing - see [here](https://git.drupalcode.org/project/smart_trim/-/pipelines/153603). But, on February 25, 2024, it was passing - see [here](https://git.drupalcode.org/issue/smart_trim-3423678/-/pipelines/103540). In both cases, "next major" was Drupal 11, no?
...a few minutes pass...
Heh, we just figured it out. It turns out all all core updates prior to 10.3 were removed from Drupal 11 on April 19, 2024 - see [here]( https://www.drupal.org/project/drupal/issues/3439769 π Remove all the pre-10.3.0 updates hooks Fixed ).
Therefore, no need for @ankitv18 to answer my question, as we have answered it for ourselves π
Bottom line - yes, we need a new fixture based on Drupal 10.3.whatever and Smart Trim 2.0.0. The easiest way to create this fixture (database dump) is probably to:
- Start with a fresh Drupal 10.3.whatever code base
- Import the existing fixture (smart_trim/tests/fixtures/update/drupal-10.0.8-smart_trim-2.0.php.gz)
- Run database updates
- Export database (as .gz, not as .zip!)
- Update line 26 of smart_trim/tests/src/Functional/Update/SmartTrimUpdateMoreTest.php
- Run test locally to ensure all is working.
- Commit changes
Did we miss anything?
-mike
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
volkswagenchick β credited ultimike β .
For me, the issue only occurs with some authenticated users. I've tried to narrow it down a bit based on permissions, but no luck.
Disabling BigPipe solves the issue for all users, so I'll be waiting for π 10.3 upgrade now missing status-message theme suggestions Postponed to be resolved as well.
-mike
With the release of Drupal 11 a few days ago, before we tag the 2.2.0 release, I'd like for us to get a fully clean GitlabCI test report. Currently, "phpunit (next major)" is failing. I don't think that there's a Drupal core 12.x branch yet (via https://git.drupalcode.org/project/drupal), so I'm not sure if we should just disable that test for now, or another option.
I'm guessing that maybe "phpunit (next major)" is running against the 11.x branch and am working to confirm this (https://drupal.slack.com/archives/CGKLP028K/p1722796036303819).
...a few minutes pass... Whoops, I just noticed π Update fixtures for run updates Active - glad we're on the same page π
Before tagging a release, I'd like to get β¨ Smart Trim Tokens for text with summary fields Needs review merged as well.
-mike
Thanks @lostcarpark and @ankitv18 - I'm closing this issue as the fix is part of β¨ Smart Trim Tokens for text with summary fields Needs review .
-mike
A couple of thoughts:
- Current tests are failing.
- If we want to proceed with this change, we'll need a new test for this new functionality.
- Do we want to proceed with this change, our goal has been to simplify the configuration, not add additional options. Is there any way to achieve the same results by utilizing a template override?
-mike
Looks good do me - merging.
ultimike β made their first commit to this issueβs fork.
I can confirm that using Gin with Field Group results in gin's tabs.css file being loaded in the front end (due to the Gin admin menu, I assume) and messing up many (all?) of Field Group's horizontal tab styles. π
-mike
I changing this to "needs work", but for any serious reason - only because I have a question about the test logic that I couldn't figure out on my own...
thanks,
-mike
@markie - I'm don't follow - did you actually merge this? I don't see where you did...
-mike
ultimike β created an issue.
Thanks everyone for pushing this issue forward, and also for your patience for one of this module's slacker maintainers finds the time to check it out (present company included).
First off - this needs a test. @markie and I have been really trying to require tests for all feature additions and bug fixes, so this would be no exception. I think a functional test would be pretty straight-forward...
I manually tested this using Gitpod with a few of our DrupalEasy alums and all appears to work as expected. The only thing I would make note of (possibly with an update to the README and/or docs) is the fact that the Token view mode must be enabled and configured with a Smart Trim formatter (for the field(s) in question) or the Smart Trim tokens aren't really involved.
Regardless, we tested Smart Trim tokens with Pathauto and Metatag with everything working as expected.
I went ahead and committed @heddn and @akalata's suggestions, as well as a PhpStan ignore for renderPlain() in smart_trim.tokens.inc (see π Handle renderPlain() deprecation Active ).
-mike
ultimike β created an issue.
ultimike β made their first commit to this issueβs fork.
volkswagenchick β credited ultimike β .
I left a couple of minor suggestions above.
-mike
I tested this using DrupalPod (Drupal core version 10.0.11, PHP 8.2) and when I went to /admin/people/createautologinurl I immediately received an error (partial stack trace):
The website encountered an unexpected error. Please try again later.
AssertionError: Cannot load the "user" entity with NULL ID. in assert() (line 261 of core/lib/Drupal/Core/Entity/EntityStorageBase.php).
Drupal\Core\Entity\EntityStorageBase->load(NULL) (Line: 106)
Drupal\auto_login_url\Form\CreateAutoLoginConfigForm->buildForm(Array, Object)
call_user_func_array(Array, Array) (Line: 536)
Drupal\Core\Form\FormBuilder->retrieveForm('auto_login_url_create', Object) (Line: 283)
I'm pretty sure this is because the first time a user visits this form, the auto_login_url.create
config doesn't exist yet. I'm a bit confused by why this config even needs to exist - is it just so that the form remembers the previously used values? If so, then maybe a better option is using the Drupal State API.
Finally, I also want to mention that I don't understand why the password field is included if this form is only available to those with the "administer auto login url" role. It seems that with the password field present, one can only create auto login urls for themselves. Granted, maybe I am missing something, but I don't understand why the password field is part of this form - please explain.
thanks,
-mike
I meant to leave this at "Needs review".
These changes look fine to me, glad to see that the tests that exist are passing, although I would be more comfortable if there was better test coverage for this module.
Without additional test coverage, I am not comfortable committing these changes based on my eyeballs alone, so I'd prefer to see additional reviews on this change.
-mike
@liberatr - thanks for posting this.
I checked this out during DrupalEasy Office Hours with some of our alums and found the idea interesting.
I was interested in checking it out as something I might be able to use on the soon-to-be-updated DrupalEasy Academy web site. I use the Group module for our "terms" ("Spring 2024 DCO", etc...) and having a nicer interface to add content (lessons, screencasts, etc...) to each term is something I'm definitely interested in.
But, as you probably already know, your model is limited in that the group name/id cannot be passed into it - it must be hard-coded as part of the model. Which means, if I have 120 terms, then I need 120 models - this is less-than-ideal.
We couldn't brainstorm a way to get around this limitation using only ECA and the other modules you use above. In fact, I think it would probably be easier to just create a custom VBO "Add to group" action where a form is presented to the user to select the group to add the content to.
Anywho, thanks for sharing this - always great to see cool stuff that ECA can do.
thanks,
-mike
@hestenet,
I'm just re-reading your comments above - quick question - does the award have to be a "per-month" thing? Can it just be a one-time award?
-mike
Changes look good to me!
thanks,
-mike
ultimike β created an issue.
@sanket.addweb - thanks for your efforts on this. Moving back to "Needs work" because we need to add at least one test for this. I would like to see a test that attempts to set the "Trim units" field to a non-integer and maybe also a test that covers the #required bit.
-mike
ultimike β created an issue.
@eleonel Thanks for the assistance.
After updating everything, I was now able to get a valid response using:
- API key: OpenAI
- SDK to use: openai-php/client
- Model: gpt-4o
But, when I changed the "SDK to use" to "orhanerday/open-ai", I received the error, "{"error":"Error during the chat completion execution, please check the logs for more information."}". Looking at the log, it has the same error as I originally reported:
OpenAI API error: We could not parse the JSON body of your request. (HINT: This likely means you aren't using your HTTP library correctly. The OpenAI API expects a JSON payload, but what was sent was not valid JSON. If you have trouble figuring out how to fix this, please contact us through our help center at help.openai.com.).
So, it looks like it is/was an SDK issue.
Thanks!
-mike
@dinazaur - I used the folder icon previously, but went back in and redid the configuration but still no luck.
I tried the following and didn't get any dropdown options for available fields:
$node = \Drupal::entityTypeManager()->getStorage('node');
$node->get('')
Can you confirm that the extension works with the latest version of PhpStorm?
thanks,
-mike
tekNorah β credited ultimike β .
tekNorah β credited ultimike β .
ultimike β created an issue.
ultimike β created an issue.
@osab - Thanks for your response. The fact that a custom cache key causes the render array to be cached independently of the rest of the page is helpful and explains what I'm seeing on /cache-review/cached-max-age
As for the naming, I think that in the Cache Review sample pages, using the word "section" is much better than using "blocks", IMHO.
-mike
ChrisDarke β credited ultimike β .