Account created on 24 February 2012, over 13 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States jcandan

@markdc, your update in #5 is different from how the help text currently describes the expected storage behavior:

Complete Split: Configuration listed here will be removed from the sync directory and saved in the split storage instead. Modules will be removed from core.extension when exporting (and added back when importing with the split enabled.). Config dependencies are updated and their changes are recorded in a config patch saved in in the split storage.

But, yes, I am also seeing this behavior--complete-split-enabled modules are listed in the sync directory's core.extension rather than in the split storage.

On first export, after configuring a complete split, I noticed that and that there were a few configuration .yml files for those modules in the split storage directory.

Based on the help text, expecting this was incorrect, I attempted to get them corrected by uninstalling the modules and re-trying the config-export. But this just removed the configuration .yml files from the split storage directory. Given it was now an empty directory except for the generated .htaccess, I deleted the directory and drush config:export again. The .htaccess file reappeared, still with no other files.

It does seem that the new behavior works as expected: config import correctly enables the modules listed in the active split. But, that the help text (and perhaps docs) need to be updated, and I don't know if the now-missing single-export files that were listed in the split storage directory need to be there.

🇺🇸United States jcandan

Doesn't seem that MR !412 quite worked. This MR pipeline list still shows a merge train pipeline. Maybe I didn't implement the CI variables to capture this fork's branch reference correctly. Not sure.

🇺🇸United States jcandan

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

🇺🇸United States jcandan

jcandan changed the visibility of the branch 3550843-fix-lone-name-validation-errors to hidden.

🇺🇸United States jcandan
🇺🇸United States jcandan

jcandan created an issue.

🇺🇸United States jcandan
🇺🇸United States jcandan

jcandan created an issue.

🇺🇸United States jcandan

jcandan created an issue.

🇺🇸United States jcandan

jcandan created an issue.

🇺🇸United States jcandan

jcandan created an issue.

🇺🇸United States jcandan
🇺🇸United States jcandan
🇺🇸United States jcandan

jcandan created an issue.

🇺🇸United States jcandan

Patch #13 applied successfully to 4.0.0-alpha6 and resolved the error for me.

🇺🇸United States jcandan

Seems you've done more work on 🐛 Errors after update to alpha6 Active since May 22nd. I propose we keep this marked duplicate to avoid confusion.

🇺🇸United States jcandan

So far as I can tell, @group demo_b and @group demo_c should have had tests. I'm not quite sure what to make of this. The fix seems right, but with these 2 groups' tests not running, I am leaving as needs work.

🇺🇸United States jcandan

I'm setting to Needs work. I just saw that 2 of the examples show "No tests executed." Might be on purpose. I am looking more closely.

🇺🇸United States jcandan

@jonathan1055, Excellent! Well done. pipeline #2051348583 shows MR 408 adds Recipe support nicely.

You didn't put this to Needs Review. Are you expecting to do more work or change anything? Based on the above, I am putting in RTBC, but feel free to change if you'd planned additional changes.

🇺🇸United States jcandan

I know it is strange, but recipes are not expected to be in separate directories (contrib/custom). If the desire here is to be able to easily adopt GenericRecipeTestBase , then the path really should be ../recipes/{$name} . Otherwise, we'd need to manipulate installer-paths.

@jonathan1055, yes, feel free to submit an MR. You'll probably need to do something like:

.composer-base:
  variables:
    DRUPAL_GITLAB_TEMPLATES_PATCH_URL: "https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/408.patch"
    GITLAB_TEMPLATES_PACKAGE: "drupal/gitlab_templates"
    GITLAB_TEMPLATES_PATCH_DESC: "Apply MR408 to gitlab_templates"
  before_script:
    - |
      composer config --no-plugins --no-interaction allow-plugins.cweagans/composer-patches true || true
      composer require --no-interaction cweagans/composer-patches || true
      composer config --no-interaction \
        "extra.patches.${GITLAB_TEMPLATES_PACKAGE}.\"${GITLAB_TEMPLATES_PATCH_DESC}\"" \
        "${DRUPAL_GITLAB_TEMPLATES_PATCH_URL}"
🇺🇸United States jcandan

@fjgarlin, I meant to clean that up. Not really, it just adds hidden-variables, additional security scans, and changes workflow rules.

@jonathan1055, the default value, no. Docs indicate it will default to modules/custom. But, if you meant that it would get it from the PROJECT_TYPE, it seems this value comes from the *.info.yml, which Recipes don't have.

Since supplying the type would place it in recipes/custom, one might be inclined to think the easy solution would be:

variables:
  PROJECT_TYPE: 'recipe'
    ...
    return "{$absolute_path}/web/recipes/custom/{$dirname}";

But, if you only specify the PROJECT_TYPE: 'recipe' CI variable, a dev would also need to update the Composer installer-paths (perhaps via .composer-base, maybe). Because, otherwise, dependencies will be in ../recipes, and he would get a recipe does not exist.

Here's an demo of this:
https://gitlab.com/jcandan/demo_drupal_gitlab_templates_recipe/-/merge_r...

So, would you like to change scope for this ticket to update documentation with an explanation about using DRUPAL_PROJECTS_PATH: '../recipes'?

🇺🇸United States jcandan

Until a solution is released, the following workaround seems to suffice:

# .gitlab-ci.yml
include:
  - project: 'blue-hangar/project/gitlab_templates'
    file:
      - '/includes/include.bluehangarci.main.yml'

variables:
  DRUPAL_PROJECTS_PATH: '../recipes'

/**
 * @file recipes/myrecipe/tests/src/Functional/GenericTest.php
 */

declare(strict_types=1);

namespace Drupal\Tests\myrecipe\Functional;

use Drupal\Tests\system\Functional\Recipe\GenericRecipeTestBase;

/**
 * @group myrecipe_recipe
 */
class GenericTest extends GenericRecipeTestBase {

  protected function getRecipePath(): string {
    // Assume this test in located in PROJECT_DIR/tests/src/Functional.
    $absolute_path = dirname((new \ReflectionObject($this))->getFileName(), 4);
    $dirname = basename($absolute_path);
    return "{$absolute_path}/recipes/{$dirname}";
  }

}
🇺🇸United States jcandan

jcandan created an issue.

🇺🇸United States jcandan

I can also confirm this fixes the error as seemingly introduced by 🐛 Controlled-by fields inside a Paragraph don't work Needs work .

🇺🇸United States jcandan

My bad: tests/src/FunctionalJavascript/ConditionalFieldParagraphsTest.php. Since there was no .gitlab-ci.yml entry to get Paragraphs, I wrongly assumed that there were no tests. I didn't realize Paragraphs was a dev-dependency for this module.

🇺🇸United States jcandan

Closing in lieu of prioritizing Support plugin integration Active .

🇺🇸United States jcandan

jcandan changed the visibility of the branch 3454164-support-compound-fields to hidden.

🇺🇸United States jcandan

Needs to be re-worked with the recent merge from 🐛 content_moderation support Needs work . I'm giving it a go.

🇺🇸United States jcandan

Added tests. Tests pass, but since the PHPUnit update bug they fail.

Needs review.

🇺🇸United States jcandan

This may prove more complex than originally thought. See #3126210-3: Address with default country code always validates as required .

I can remove the HTML5 required attribute via any number of hooks, but backend validation still applies and the fields are required on submit (published or not).

I'm glad we separated this from Add support for multi-value fields like Name and Address fields Active . In the meantime, as a not-so-great workaround, those wishing to apply Require on Publish to an Address field may consider leaving off a default value, and assuming the user's intent to fill out the full address when they provide a country.

🇺🇸United States jcandan

Moving forward with the release. 🐛 content_moderation support Needs work will come in a patch shortly thereafter.

🇺🇸United States jcandan

Released 2.0.0-rc2!

Very happy with the tests added in 🐛 content_moderation support Needs work . Very strong candidate for 2.0.0. That ticket needs review.

🇺🇸United States jcandan

Added significant test coverage including Paragraphs and Content Moderation support. Needs review. Any takers?

🇺🇸United States jcandan

Needs test coverage.

🇺🇸United States jcandan

Bumps previous phpunit job to this phpunit job from 8 to 9 tests and 73 to 77 assertions. Happy with the result.

🇺🇸United States jcandan

Given the jump in assertions, I consider this a success.

🇺🇸United States jcandan

Never mind. This only becomes a problem when we introduce #states Form API handling. Providing the third-party setting programmatically is the correct path forward for tests in Add support for multi-value fields like Name and Address fields Active . Postponed until we work on Add require on publish as a conditional field option Active .

🇺🇸United States jcandan

@anirudhsingh19, thank you for contributing! We're close. We need test coverage. I got us part of the way there by adding Name as a part of the CI pipeline.

However, I recognized a problem while writing tests for this. We're currently blocked by 📌 Ensure require_on_publish setting is applied when FieldConfig is created programmatically Active .

🇺🇸United States jcandan

My bad--I didn't explain why I'd switched the version. It seems 2.x-dev should be deleted while a minor dev releases exists. See 🐛 Fix Composer stable, caret constraint installs dev release Active .

@jeroen dost, I agree this affects both, but it seems--as with many other issues-- 📌 Refactor custom JS for CKEditor5 v45+ Active will address this issue. I wonder if we need to ensure the scope of that ticket includes this, or just mark this as a 2.x-dev issue.

I can see an argument for either way. Just having this note here may be sufficient. :)

🇺🇸United States jcandan

Also, in addition to the problems described, it seems 2.x has been abandoned anyway (source).

🇺🇸United States jcandan

Pipelines fail. Needs fixes. Would like to shoot for a 2.1.0 release.

🇺🇸United States jcandan

Am considering this for 2.0.0, or 2.0.1, depending on the speed with which we get a review.

🇺🇸United States jcandan

We have released the bug fixes to 8.x-1.11 and 2.0.0-rc1.

We encourage everyone to at least try 2.0.0-rc1. Feel free to provide feedback here or open new bug tickets.

We will support bug fixes8.x-1.x for at least a couple of weeks after 2.0.0 is released, which we hope to complete in the coming days.

🇺🇸United States jcandan

Also...

so a composer install on a fresh environment no longer works to patch

@alimc29, I wouldn't rely on MR URLs for patches. It is best to use patch files uploaded to the issue or to commit them to your project.

However, don't fret--a release candidate supporting Drupal 11 is forthcoming. :)

🇺🇸United States jcandan

Yeah, sorry, this is in progress. Assigned to me. I am working out some testing kinks in 🐛 Ensure ConfigPageTest actually verifies require_on_publish setting Active first. I plan to get this out today.

🇺🇸United States jcandan

Could we add the following to one of the lists?
🐛 Empty action sets field value to "undefined" Active

🇺🇸United States jcandan

Tested 2.3.1 on Drupal 11.2. This issue does not exist in 2.3.1.

This bug is isolated to 2.2.x releases, and should be addressed in that branch for continued support up to Drupal 10.4 and 11.1 and below core versions.

Drupal 10.4 and 11.1 will receive security coverage until December 2025 ( source ).

Production build 0.71.5 2024