🇺🇸United States @pfrilling

All tests passed after the rebase. I went ahead and merged this.

Thanks everyone!

I rebased the code with the 3.x version to get the tests validating Drupal 10 and 11. If all tests pass, I'll get this merged.

I rebased the branch and updated the merge request to include functional tests. Marking this as needs review for someone to confirm my updates.

pfrilling → made their first commit to this issue’s fork.

Thanks all!

This has been merged to 3.x.

I added unit tests to the new methods and I renamed two of the methods to match the other save/retrieve token patterns:

- retrieveTokenExpire was renamed to retrieveExpireToken
- saveTokenExpire was renamed to saveExpireToken

pfrilling → made their first commit to this issue’s fork.

The changes proposed here were out of date so I:

- rebased with the latest 3.x
- fixed all phpcs linting errors
- fixed all cspell errors
- fixed all phpstan errors

Would love someone to review my edits to confirm my changes look okay.

pfrilling → made their first commit to this issue’s fork.

Attached is a patch that I started. Not entirely sure I'm on the right path. Would love some feedback and/or it can help someone else as a starting point.

I believe I addressed everything. Please review again.

I addressed the MR feedback. This is ready to review again.

Thanks for the fix. I agree, this logging should not be included.

I updated the tests to pass without actually getting a successful submission. Not sure how to get Cypress to bypass and/or pass the Captcha element?

All new code should be under test coverage now.

I changed the branch to 1.x. I also changed the status back to needs work to get the build errors addressed.

I added unit tests to the ActionsForm class as well as the Drush command.

I think we still need test coverage on the clearStorage() method.

pfrilling → made their first commit to this issue’s fork.

Code looks good to me.

I believe the intent is to move the contact form into a layout builder section of a basic page so that additional content can be included along with the form. Assuming that is the direction, the form id will never be consistent and will always include a node id. Inserting the captcha as a form element instead of a captcha point will allow the captcha to be present without needing to know a node id.

The other option would be to have the captcha point utilize the base form id, which should include all derivatives of the contact form placed in a layout builder section.

I did a quick test of the latter option by adding the base form id `webform_submission_contact_form_form` as a captcha point and placing the contact form in a layout builder section on a node. When browsing that node anonymously, the captcha did show.

So, either of the above options should work. I guess it might come down to editor experience at that point... does it make sense to have a captcha element on each form like any other form field or does it make sense to route an editor to another configuration page to add a captcha point?

I added a config action to the Webform project here https://www.drupal.org/project/webform/issues/3490313#comment-15877907 ✨ Add Config Action to enable captcha form elements Active that will add a `captcha` form element. With this, we'll be able to add the captcha points as a form element.

I have some code to work with that plugin going locally. I'll work on getting it pushed for further testing.

I added the config action that will add a captcha form element to a form if it does not yet exist.

pfrilling → created an issue.

I rebased the code and confirmed all tests are now passing. Marking as needs review.

Working on the rebase now.

I was able to get the tests all passing. Marking this as needs review.

I reviewed the code and the yml changes look correct to me. Marking this RTBC.

The markup is changing in https://www.drupal.org/project/project_browser/issues/3485747 🐛 The multi-category filter needs to be an actual set of labeled checkboxes Active . This will likely need a rebase once that fix lands.

The markup changes have been completed and the testing is all passing. This is ready for review.

Looking into this at NEDCamp. Adding the issue tag.

pfrilling → made their first commit to this issue’s fork.

johnpicozzi → credited pfrilling → .

Adding the nedcamp2024 tag.

The code looks good to me. All the MR feedback looks to have been addressed.

I added functional tests and confirmed this is working.

pfrilling → made their first commit to this issue’s fork.

Simplenews

• This module allows an administrator to create a newsletter entity to which people are able to subscribe.
• Subscriptions are managed with a Drupal block that is placed in a region.
• After a user subscribes, they are sent a confirmation message with a link back to the site. Once they follow those steps, they are then 'subscribed'.
• An administrator then can create a newsletter issue, and assign it to a newsletter. Once the newsletter is ready to send, the administrator can browse to a page and send the message to all those subscribed. Drupal's cron process will then handle sending to all subscribers.

Questions

If we decide to go this route, I think we need to determine the following:

1. What newsletter should be included?
2. What text should the subscription block contain and where should it be placed?
3. What should the confirmation messaging be?
4. What roles should we create to allow the newsletter creation/sending?

pfrilling → created an issue.

I believe I addressed everything. This is ready for another review.

I addressed all the feedback in the MR.
- I decided to list out all of the webform configuration instead of archiving the one form that shipped with Webform.
- I reverted any changes I made to the anti spam recipe. I'll create a followup issue to address changes in #3477309
- I did add a cypress e2e test, which passes locally, but not in CI. I don't think the E2E testing is configured quite right in the CI. It seems that the site is not getting installed in that CI environment.

I think this is ready to review again.

I fixed the obvious things and left comments for a few others. Changing this to needs review for visibility, but I'm sure more work will follow.

Addressed the MR feedback. Marking as needs review.

I addressed the feeback in the 3.x merge request and added a new functional test to validate this functionality. If someone can check the code/test the functionality, I can work on getting this merged.

pfrilling → made their first commit to this issue’s fork.

Tests have been added to confirm that the UI fixtures are validating against the Open API Spec. This should resolve #3.

3. Test that the fixtures in ui/src/mocks/fixtures match the OpenAPI spec, which proves the client (XB React UI) conforms to the OpenAPI spec

This has been resolved and the API Spec test should be passing now.
> # 1 The new test is failing:

Do you have any examples for testing the fixtures against the spec? If not, I'll start researching how to make that happen.

Looking through the handlers mock, I started the attached OpenAPI spec. It's still a work in progress, but I think it is a good starting point. Let me know what I missed and I can continue updating the spec.

I created a merge request and confirmed that the CKEditor loads correctly and the image popup seems to be working properly.

I'm leaving this as needs work since it will require an automated test.

pfrilling → made their first commit to this issue’s fork.

pfrilling → made their first commit to this issue’s fork.

Created a merge request to address the missing cache tags for the configuration.

pfrilling → created an issue.

I'm looking at this today.

Added the test confirming that the clear button is not focused on tabbing.

I'm working on this during Portland2024.

Marking fixed as the merge train has begun.

Looks good.

This was fixed with #3409686

Fixed the settings/tests to work with Drupal 10.2. This did break compatibility with Drupal 10.1.

Confirmed the behavior changed for 10.2 with the field ui improvements.

I forced the MR into the code as the pipeline failures were unrelated to the actual changes.

The code looks good to me.

Uploading a patch for use with composer.

The code changes look good to me. Marked as RTBC.

nicxvan → credited pfrilling → .

Thanks for the patches and tests! This has been committed with https://git.drupalcode.org/project/library_field/-/commit/c325cad2d5d2d6...

Un-assigning myself.

The tests in the merge request are passing again. This is ready for a review.

pfrilling → created an issue. See original summary → .

pfrilling → created an issue.

Attached is a patch that _should_ add test coverage for both warnings and errors coming from package manager. I also attempted to merge the changes from the MR into this patch. I admin, I'm no Svelte expert, so there is a good chance I didn't do the JS stuff correctly.

pfrilling → changed the visibility of the branch 3365180-pm-warnings to active.

pfrilling → changed the visibility of the branch 3365180-pm-warnings to hidden.

Beginning to work through this issue today.

This has been fixed and added to the -dev branch.

The merge request is passing manual testing. However, I'm marking this as needs work so I can add functional tests to validate.

Working on adding this feature.

pfrilling → created an issue.

johnpicozzi → credited pfrilling → .

This has been fixed and merged into the -dev branch.

pfrilling → created an issue.

Marking this RTBC based on r.aubin's comment in #5.

@r.aubin, this has been added and is ready to review in this MR: https://git.drupalcode.org/project/sfds/-/merge_requests/7

This has been fixed and merged into the 8.x-2.x-dev branch

pfrilling → created an issue.

pfrilling → created an issue.

Validation has been added to the configuration form. I'm opening a follow up task to add validation to the config entity directly, which is not currently available.

Looks good. Thanks Alex!