🇺🇸United States @pfrilling

All tests are now green!!

Thanks @phenaproxima! That did the trick!

I went ahead and fixed the phpstan warnings too. I believe this is now ready for review (I believe the Nightwatch failures are expected).

I updated the drush test to be like the examples you provided @chrisfromredfin. The new test is passing, however, I seem to have broken phpcs/phpstan checks after my rebase. Any thoughts, or do I need to revert and try again?

The php-mock library was just used to mock the global `function dt()` from Drush. The Core documentation I found for mocking these types of functions is here: https://www.drupal.org/docs/automated-testing/phpunit-in-drupal/unit-tes... → , but I honestly always feel like I'm doing something wrong when mocking the global functions that way.

My personal preferences aside, I can rewrite the tests to remove that extra package if you feel strongly about it.

This sounds like a similar issue as #3487116, which should have corrected this.

- What OpenID Client are you using?
- Can you provide the plugin's configuration before and after the upgrade (obfuscating the sensitive parts)?

Looks good. I believe they were all addressed.

A Drupal user account that gets provisioned through any OpenID client should be considered a federated member. Thus, the Identity Provider (IDP) which provisioned the user should be considered the canonical source for the member's data. Any updates to the member's data should be done on that platform.

In order to allow for changes to account data within Drupal, I believe a federated member should disconnect their IDP and then follow a password reset process.

I am in support of disabling the member's email/password fields in these situations. We could probably take it a step farther and provide an action that disconnects them from their IDP and then initiates a password reset to the email stored in the Drupal user account. Then, they could change their password and reconnect to an IDP if they chose.

As for updating an IDP with updates in Drupal: I do think it would be possible to bubble account changes from Drupal back up to a trusted IDP through an API request, but I believe that is a pretty specific requirement and should live in a projects custom code. There are too many variables at play with too many different API requests for it to live here.

From the reproduction steps that @joelfp I would consider this working as designed. Browsing between a browser session and incognito should never work because of the requirement of the session.

- One thing to check is that a member has the `openid connect set own password` if you want to reset a password.
- What is the IDP that you are using?

I think we need more information and solid reproduction steps to consider if this is a bug or not.

I was able to write a functional test to confirm this fix is working. During the automated testing, I discovered that the schema for empty scopes is incorrectly storing a string instead of an empty array. I updated the submit configuration form to properly save an array to configuration, but left the other fixes in place for backwards compatibility.

I'm going to create a followup issue for the other provided clients to get those saving the configuration correctly.

If someone has time to review my changes, that'd be great!

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

Thanks @dcam! I made the method name change you suggested.

Since Drupal 9 is no longer supported and the form changes were already made that broke the settings form in D9, I went ahead and dropped support for that version.

Left a few comments in the MR that need to should be addressed.

Attributing credit

Attributing credit. Thanks again!

Attributing credit. Thanks again!

Attributing credit to all who helped. Thanks again!

I made the change you called out on the MR @dcam.

Thanks everyone for your work! This has been merged.

There is an End Session endpoint configuration setting for each client that is working correctly in my testing. I'm closing as cannot reproduce. Please reopen if you are still experiencing issues.

After reviewing the description, I believe what you are looking to accomplish is currently being tracked in this issue: https://www.drupal.org/project/openid_connect/issues/3011413 ✨ Autologin when one client enabled Needs work

I'm going to close this as a duplicate.

I updated the tests to:

1. Fix the externalauth schema error
2. Added a test case that ensures an existing role mapping remains.
3. Made the fixture a bit easier to read.

I believe this all looks good. Would love someone to review my changes before I commit.

Assigning myself as I review the issue.

I'm not sure where the Guzzle client would be used instead of the httpClient provided by Drupal. Can you provide more details as to where you are seeing this issue?

It seems I already made this change in #3452009 (https://git.drupalcode.org/project/openid_connect/-/merge_requests/110/d...).

That being said, I think dropping support for Drupal 9 is acceptable being that it has not been supported for over a year now. Would you agree?

I updated the merge request to add a test to confirm the settings form is loading on Drupal 10 and 11.

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

Thanks all! This has been merged to 3.x.

pfrilling → created an issue.

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...