Merged!
Looking good to me.
mparker17 → created an issue.
(fix copy-paste typo in issue description)
(fix copy-paste typo in issue description)
(fix copy-paste typo in issue description)
mparker17 → created an issue.
(update issue summary to mention Geofield, explicitly mention adding a test, and updating the documentation → )
mparker17 → created an issue.
(update the issue summary for consistency with similar tickets for other module integrations)
mparker17 → created an issue.
mparker17 → created an issue.
mparker17 → created an issue.
Change "Testing..." section title to "Test case" for better automatic anchor ID generation, for better linking. Simplify Setup section intro; remove unneeded manual anchors
Change "Testing..." section title to "Test case" for better automatic anchor ID generation, for better linking. Simplify Setup section intro; remove unneeded manual anchors
Change "Testing..." section title to "Test case" for better automatic anchor ID generation, for better linking. Simplify Setup section intro; remove unneeded manual anchors
Change "Testing..." section title to "Test case" for better automatic anchor ID generation, for better linking. Simplify Setup section intro; remove unneeded manual anchors
Link to Search API Decoupled test case; change "Testing..." section title to "Test case" for better automatic anchor ID generation, for better linking.
Change "Testing..." section title to "Test case" for better automatic anchor ID generation, for better linking.
Add note contrasting the Search API Decoupled test case.
Add note contrasting the JSON:API Search API test case
+1 to RTBC: I have code-reviewed and manually tested the code in merge request !4 and I'm satisfied with it.
Here's what I did to manually-test !4:
- Install ddev (I tested on version 1.24.1)
- Clone the module, issue fork, and branch:
git clone --branch '2.x' https://git.drupalcode.org/project/blockgroup.git && cd blockgroup
- clone the projectgit remote add blockgroup-3429024 https://git.drupalcode.org/issue/blockgroup-3429024.git && git fetch blockgroup-3429024
- add a remote for the issue forkgit checkout -b 'project-update-bot-only' --track blockgroup-3429024/'project-update-bot-only'
- switch to the branch for merge request !4
- Set up a test site for the module with the ddev/ddev-drupal-contrib plugin:
ddev config --project-type=drupal --docroot=web --php-version=8.3 --corepack-enable --project-name=blockgroup
ddev add-on get ddev/ddev-drupal-contrib && ddev start && ddev poser && ddev symlink-project
- run the setup steps for the ddev/ddev-drupal-contrib addon- Go to
https://blockgroup.ddev.site/core/install.php
in a browser. Confirm that I saw the Drupal 11.1.1 installer. Install the site with the "Standard" install profile - Go to
/admin/people/create
and create a user with the Content editor role. I used the usernametest_editor
- Set up the module for testing:
- Go to
/admin/modules
; enable the blockgroup module and all its dependencies. - Go to
/admin/people/permissions
and grant the Content editor role permissions to:- Block -> Administer blocks
- Block Content -> Basic block: Create new content block
- Block Content -> Basic block: Basic block: Edit content block
- Block Content -> Access the Content blocks overview page
- Block Content -> Administer block content
- Block Group -> Administer blockgroups
- Go to
/block/add
. You see an Add content block form:- Set Block description to
Block A title
. - Set Body to
Block A body
. - Click Save.
You see the message Basic block Block A title has been created.
- Set Block description to
- Go to
/block/add
. You see an Add content block form:- Set Block description =
Block B title
. - Set Body =
Block B body
. - Click Save.
You see the message Basic block Block B title has been created.
- Set Block description =
- Go to
- Test the basic module functions in Drupal 11:
- Log out of the administrator account, and log in as
test_editor
created earlier - Go to
/admin/structure/block_group_content/add
. You see an Add group form. Set Label =Test blockgroup
. Click Save. You see the message Created the Test blockgroup Block group content. - Go to
/admin/structure/block
. You see a Block layout form. You see a region with the title Block group: Test blockgroup.- Click Place block beside the Block group: Test blockgroup region title. You see a Place block modal.
- Click the Place block button in the
Block A title
row. You see a Configure block form. Ensure that Region =Block group: Test blockgroup
. Click Save block. - A Block A title Content block appears in the Block group: Test blockgroup region. If you scroll up, you see the message The block configuration has been saved.
- Click Place block beside the Block group: Test blockgroup region title. You see a Place block modal.
- Click the Place block button in the Block B title row. You see a Configure block form. Ensure that Region =
Block group: Test blockgroup
. Click Save block. - A Block B title Content block appears in the Block group: Test blockgroup region. If you scroll up, you see the message The block configuration has been saved.
- Finally, scroll to the bottom of the form and click Save blocks. You see the message The block settings have been updated.
- Go to
/user
. You see a user/view page for test_editor. You do not see the textTest blockgroup
anywhere on the page. You do not see the textBlock A title
anywhere on the page. You do not see the textBlock A body
anywhere on the page. You do not see the textBlock B title
anywhere on the page. You do not see the textBlock B body
anywhere on the page. - Go to
/admin/structure/block
. You see a Block layout form. You see a region with the title Sidebar.- Click Place block beside the Sidebar region title. You see a Place block modal.
- Click Place block in the
Test blockgroup
row. You see a Configure block form. Ensure that Region =Sidebar
. Click Save block. - A Test blockgroup Block Group block appears in the Sidebar region. If you scroll up, you see the message The block configuration has been saved.
- Finally, scroll to the bottom of the form and click Save blocks. You see the message The block settings have been updated.
- Go to
/user
. You see a user/view page for test_editor. You see the textTest blockgroup
in the sidebar. You see the textBlock A title
below it. You see the textBlock A body
below it. You see the textBlock B title
below it. You see the textBlock B body
below it.
- Log out of the administrator account, and log in as
- Make sure there are no errors being logged:
- Log out of the
test_editor
user created earlier; and log in as the administrator again. - Go to
/admin/reports/dblog
to ensure our setup/testing did not result in any error messages related to this module.
- Log out of the
Typo: "Hide reset link when no facet is selected" -> "Hide reset link when no facet item is selected"
Make editing the facet easier to follow. Fix missing code block in URL for placing the facet block.
Adding 🐛 Merging Multiple Facet Conditions in facets_post_filters with OR Conjunction in Elasticsearch Connector Active as a blocker.
Realizing this will make testing 🐛 Merging Multiple Facet Conditions in facets_post_filters with OR Conjunction in Elasticsearch Connector Active easier, so I'm going to merge it now.
Update Setup for working on an issue after merging 📌 Make 8.0.x a bit easier to test Active .
Are you comfortable writing automated tests?
I should probably clarify: I fixed the test that broke after applying your patch (i.e.: changing tag
to tags
in the constructed query).
... But a test for the exact scenario you were running into in the issue description doesn't exist yet, and we should create one, so that the behavior that you rely on doesn't regress in the future!
(housekeeping: now that there's an issue fork, I'm hiding the old patch; and I'm moving the issue to Needs work
and adding the tag Needs tests
)
@kir lazur, thank you very much!
I've created a merge request from your patch and credited you as the author.
Are you comfortable writing automated tests? If not, I'd be happy to help!
mparker17 → made their first commit to this issue’s fork.
These changes have been working quite well for me as I've been writing the following sets of documentation:
I've created some follow-up issues to track the omissions identified in the previous comment, so I'm going to close this now.
mparker17 → created an issue.
mparker17 → created an issue.
mparker17 → created an issue.
Update some typos; explain how to get to the test view.
@prashant mishra, did you intend to close the merge request without merging?
Made example queries more compact; fixed typo in the last one
Converted list of useful requests from unordered list to table
Delete the broken table in the Useful requests section
Hi @jwilson3, I have, and it appears to work correctly!
Here is what I did to test...
- Install ddev (I tested on version 1.24.1)
- Clone the module:
git clone --branch '2.0.x' https://git.drupalcode.org/project/label_help.git && cd label_help
- Set up a test site for the module with the ddev/ddev-drupal-contrib plugin:
ddev config --project-type=drupal --docroot=web --php-version=8.3 --corepack-enable --project-name=label-help
- create a ddev project for testing the moduleddev add-on get ddev/ddev-drupal-contrib && ddev start && ddev poser && ddev symlink-project
- run the setup steps for the ddev/ddev-drupal-contrib addon- Go to
https://label-help.ddev.site/core/install.php
in a browser. Note that I saw the Drupal 11.1.0 installer. Install the site with the "Standard" install profile.
- Set up the module for testing:
- Go to
/admin/modules
and install label_help - Go to
/admin/structure/types/manage/page/fields/node.page.body
, and see the Label help message textarea. Set Label help message toThis is a test of the label help message
. ClickSave settings
.
- Go to
-
Test the basic module functions in Drupal 11:
- Go to
/node/add/page
. I see a Create Basic page form with an Title and Body field. The text This is a test of the label help message appears below the Body field label (please see the attached screenshot).
- Go to
- Make sure there are no errors being logged:
- Go to
/admin/reports/dblog
to confirm that the module setup/testing did not result in any error messages related to this module.
- Go to
I've been working on https://www.drupal.org/docs/contributed-modules/elasticsearch-connector/... → as a way to document the elasticsearch_connector/search_api_location module integration.
Regarding data storage/mapping, as of commit 9cc6fea to the 8.0.x branch:
- It is possible to set up a Search API Field to map to an ElasticSearch geo_point field type. (e.g.: follow the instructions in the document to
Configure the module for testing →
, then run
curl 'https://ec8.ddev.site:9201/test_store_search/_mapping?format=json' | grep '"field_location":{"type":"geo_point"}'
- It is not yet possible to map a Search API Field to an ElasticSearch geo_shape field type. I daresay this should be split into another issue at lower priority.
Regarding searching, as of commit 9cc6fea to the 8.0.x branch:
- It does not yet appear to be possible to use Search API Location / Search API Location Views to perform ElasticSearch Geo queries. I daresay this should be split into another issue.
Regarding faceting (Bucket aggregation), as of commit 9cc6fea to the 8.0.x branch:
- It does not yet appear to be possible to aggregate search results by Geo-distance, Geohash grid, Geohex grid, Geotile grid, Geo-line, Geo-bounds, or Geo-centroid. I daresay this should be split other issue(s) at lower priority.
Add instructions for configuring a Content type, 2 pieces of test Content, an Elasticsearch-backed Index with Location content, and the start of a search view.
Simplify the Setup -> Extended version steps now that JQ is mentioned in the prerequisite setup page.
Simplify the Setup -> Extended version steps now that JQ is mentioned in the prerequisite setup page.
Consistent instructions to proceed to configuration in Setup section, Expanded version.
Consistent instructions to proceed to configuration in Setup section, Expanded version.
Note that the test case assumes that the test content from the test module search_api_test_example_content
has been imported.
Previous change fixed several copy-paste errors but introduced another
Add a weight of -10 so this is (hopefully) the first page in the guide
Add a section requesting that people not commit the changes to composer.json
needed as a temporary work-around for ddev/ddev-drupal-contrib
.
Moved under the Developing Elasticsearch Connector 8.0.x guide.
Clarified that this is for development (not testing), and that the instructions in this guide do not reflect best practices for a production environment!
Awesome, thanks @prashant mishra!
For my own reference, in the merge request, the updated Setup for working on an issue is...
# Switch to the issue fork here
ddev symlink-project
ddev drush -y site:install minimal
ddev drush -y pm:install toolbar field_ui views_ui
ddev drush -y theme:install claro
ddev drush -y config:set system.theme admin claro
ddev drush -y pm:install elasticsearch_connector_test
ddev drush -y elasticsearch_connector_test:add-test-content
ddev drush -y search-api:index test_elasticsearch_index
ddev drush -y cr
ddev launch $(ddev drush uli)
And there is now a search view at... /test-elasticsearch-index-search
mparker17 → created an issue.
Test content from search_api_test_example_content is not installed by default.
Include commands for how you can set up elasticsearch_connector_test submodule and reindex the test content, to make it easier to get up and running.
(typo in issue summary, thanks for your patience with me!)
(meant to use an OL in the issue summary, not a UL)
Maintainers, please let me know if there is anything that I can do to help!
mparker17 → created an issue.
Added as short version for Setup for working on an issue, explained how to add helper modules.
I've reviewed the code changes in the merge request, and it looks good to me.
I ran some manual tests (documented below) of the basic functionality on the code in !4 and Drupal 11, and I was able to get it to work.
Here's what I did to test !4...
- Install ddev (I tested on version 1.24.1)
- Clone the module, issue fork, and branch:
git clone --branch '2.x' https://git.drupalcode.org/project/masquerade_field.git && cd masquerade_field/
- clone the projectgit remote add masquerade_field-3495033 git@git.drupal.org:issue/masquerade_field-3495033.git && git fetch masquerade_field-3495033
- add a remote for the issue forkgit checkout -b '3495033--drupal-11' --track masquerade_field-3495033/'3495033--drupal-11'
- switch to the branch for merge request !4
- Set up a test site for the module with the ddev/ddev-drupal-contrib plugin:
ddev config --project-type=drupal --docroot=web --php-version=8.3 --corepack-enable --project-name=masquerade-field
- create a ddev project for testing the moduleddev add-on get ddev/ddev-drupal-contrib && ddev start && ddev poser && ddev symlink-project
- run the setup steps for the ddev/ddev-drupal-contrib addon- Go to
https://masquerade-field.ddev.site/core/install.php
in a browser. Confirm that I saw the Drupal 11.1.0 installer. Install the site with the "Standard" install profile. - Go to
/admin/people/create
and create a user with the Content editor role. I gave mine the usernametest_editor
. - Go to
/admin/people/create
and create a user with the Authenticated role. I gave mine the usernametest_user
.
- Set up the module for testing:
- Go to
/admin/modules
and install masquerade_field and its dependencies - Go to
/admin/people/permissions
, grant the Content editor role the following permissions:- Masquerade Field -> Edit the masquerade field
- Masquerade Field -> View any masquerade field
- Masquerade Field -> View own masquerade field
- User -> View user information
... click Save permissions
- Go to
-
Test the basic module functions in Drupal 11:
- Log out of the administrator account, and log in as the
test_editor
Content editor user created earlier. - Go to the edit tab for the test_editor account (on my test site, it was at
/user/2/edit
). You see a Masquerade as field. - Enter
test_editor
into the Masquerade as autocomplete, and select it from the autocomplete drop-down (on my test site, that made it saytest_editor (2)
. - Click Save at the bottom of the page.
- You see the error message User test_editor cannot masquerade as itself.
- Enter
test_user
into the Masquerade as autocomplete, and select it from the autocomplete drop-down (on my test site, that made it saytest_user (3)
) - Click Save at the bottom of the page.
- You see the status message The changes have been saved.
- Click the View tab at the top of the page.
- You see a Masquerade as field, with a link to masquerade as test_user.
- Click the test_user link to masquerade as the test_user.
- You see the status message You are now masquerading as test_user.
- Click the Unmasquerade link in the site header. If you can't see it, you may need to click the Menu button in the top-right corner of the page. If you're not using the Olivero theme, the Unmasquerade link may be located elsewhere.
- You see the status message You are no longer masquerading as test_user.
- Log out of the administrator account, and log in as the
- Make sure there are no errors being logged:
- Log out of the
test_editor
user created earlier; and log in as the administrator again - Go to
/admin/reports/dblog
to ensure our setup/testing did not result in any error messages related to this module.
- Log out of the
@cb_govcms, good catch, thank you!
@nghua, thanks for fixing the issues raised by myself and @cb_govcms. (I don't have permission to mark the thread that @cb_govcms started as "resolved" however)
I've code-reviewed the updated merge-request, and I'm happy with it. I also re-ran my test case from earlier on a freshly-cloned copy of the merge request, and it works for me.
I'm going to mark this as Reviewed and Tested by the Community. Thanks everyone!
Added a short version of instructions for setting up ElasticSearch Connector 8.0.x
Awesome, thanks @heddn!
+1 to RTBC: I have code-reviewed and manually tested the code in merge request !3, and I'm satisfied with it.
Here is what I did to manually-test !3:
- Install ddev (I tested on version 1.24.1)
- Clone the module, issue fork, and branch:
git clone --branch '3.0.x' https://git.drupalcode.org/project/cloudfront_cache_path_invalidate.git && cd cloudfront_cache_path_invalidate
- clone the projectgit remote add cloudfront_cache_path_invalidate-3429249 https://git.drupalcode.org/issue/cloudfront_cache_path_invalidate-3429249.git && git fetch cloudfront_cache_path_invalidate-3429249
- add a remote for the issue forkgit checkout -b 'project-update-bot-only' --track cloudfront_cache_path_invalidate-3429249/'project-update-bot-only'
- switch to the branch for merge request !3
- Set up a test site for the module with the ddev/ddev-drupal-contrib plugin:
ddev config --project-type=drupal --docroot=web --php-version=8.3 --corepack-enable --project-name=cloudfront-cache-path-invalidate
ddev add-on get ddev/ddev-drupal-contrib && ddev start && ddev poser && ddev symlink-project
- run the setup steps for the ddev/ddev-drupal-contrib addon- Go to
https://cloudfront-cache-path-invalidate.ddev.site/core/install.php
in a browser. Confirm that I saw the Drupal 11.1.0 installer. Install the site with the "Standard" install profile - Go to
/admin/people/create
and create a user with the Content editor role. I gave mine the usernametest_editor
- Set up the module for testing:
- Go to
/admin/modules
; enable thecloudfront_cache_path_invalidate
module and all its dependencies. - Go to
/admin/people/permissions
, grant the Content editor role the following permissions:- Cloudfront Cache Path Invalidate -> Use Cloudfront Cache Invalidate Form
... click Save permissions
- Edit
web/sites/default/settings.php
, adding (and filling in values for) the following lines for the website that is cached with CloudFront, as directed in README.md:
$settings['aws.distributionid'] = ''; $settings['aws.region'] = ''; $settings['s3fs.access_key'] = ''; $settings['s3fs.secret_key'] = '';
- Go to
- Test the basic module functions in Drupal 11:
- Log out of the administrator account, and log in as
test_editor
that I created earlier - In another tab, open a page on the website that is cached with CloudFront (I will call this $TEST_PAGE_URL below — I picked a page that doesn't get much traffic). Opened my browser's Developer Tools' Network console. Refreshed $TEST_PAGE_URL until I saw an HTTP Response Header that looks like
x-cache: Hit from cloudfront
- In another tab, logged into my AWS Cloudfront UI, go to the distribution for the website that is cached with CloudFront, and then go to the Invalidations tab for that distribution. Take note of the most-recent Invalidation's Date.
- Back in my Drupal 11 test site, go to
/admin/config/services/cloudfront-invalidate-url
. I see a Cloudfront Cache Setting form. - In the URL to invalidate Cloudfront cache textarea, enter the path component of $TEST_PAGE_URL, took note of the current date/time, then click Invalidate Cloudfront Cache (e.g.:
/node/123
or/path/to/url/alias
). I see the status message Cloudfront URL Cache invalidation is in progress. - Switch to the tab with my AWS Cloudfront UI for my Distribution. Refresh the Invalidations tab for that distribution. I see a new Invalidation (i.e.: different from the previous invalidation I noted earlier). The new Invalidation's Date was the date/time that I clicked the Invalidate Cloudfront Cache button in the previous step. The new Invalidation's Object paths matched the path I entered in the previous step. The new Invalidation's Status was Completed.
- Switch to the tab where I had $TEST_PAGE_URL open. Opened my browser's Developer Tools' Network console. Refreshed $TEST_PAGE_URL: I now see an HTTP Response Header that looks like
x-cache: Miss from cloudfront
.
- Log out of the administrator account, and log in as
- Make sure there are no errors being logged:
- Log out of the
test_editor
user created earlier; and log in as the administrator again - Go to
/admin/reports/dblog
to ensure our setup/testing did not result in any error messages related to this module.
- Log out of the
I've tried, but as mentioned in #7, my D7 to D10 site migration is complete and my D7 site retired, so I have no way to test that it still worked the way it did 10 months ago.
I have code-reviewed and manually tested the code in merge request !34, and I'm satisfied with it.
Here is what I did to manually-test !34:
- Install ddev (I tested on version 1.24.1)
- Clone the module, issue fork, and branch:
git clone --branch '2.0.x' https://git.drupalcode.org/project/openid_connect_windows_aad.git && cd openid_connect_windows_aad
- clone the projectgit remote add openid_connect_windows_aad-3485376 https://git.drupalcode.org/issue/openid_connect_windows_aad-3485376.git && git fetch openid_connect_windows_aad-3485376
- add a remote for the issue forkgit checkout -b '3485376-drupal-11-support' --track openid_connect_windows_aad-3485376/'3485376-drupal-11-support'
- switch to the branch for merge request !34
- Set up a test site for the module with the ddev/ddev-drupal-contrib plugin:
ddev config --project-type=drupal --docroot=web --php-version=8.3 --corepack-enable --project-name=openid-connect-windows-aad
ddev add-on get ddev/ddev-drupal-contrib && ddev start && ddev poser && ddev symlink-project
- run the setup steps for the ddev/ddev-drupal-contrib addon- Patch openid_connect with
#3486049-6: 'Settings' option not accessible →
to fix a bug unrelated to this module:
cd web/modules/contrib/openid_connect && curl -OL https://www.drupal.org/files/issues/2024-11-11/openid_connect-3486049-6.patch && patch -p1 < openid_connect-3486049-6.patch && cd -
- Go to
https://openid-connect-windows-aad.ddev.site/core/install.php
in a browser. Confirm that I saw the Drupal 11.0.9 installer. Install the site with the "Standard" install profile
- Set up the module for testing:
- Go to
/admin/modules
and enable theopenid_connect_windows_aad
module and its dependencies - Go to
/admin/config/people/openid-connect/settings
and set the following options: (if you get an error when visiting this URL, don't forget to patch openid_connect with #3486049-6: 'Settings' option not accessible → to fix the bug in that module)- Save user claims on every login = (checked)
- Override registration settings = (checked)
- OpenID buttons display in user login form =
Above
- Advanced -> Automatically connect existing users = (checked) (warning: you probably don't want to use this setting on a real site, but it's good enough to test openid_connect_windows_aad's basic functionality in D11)
... then click Save configuration.
- Go to
/admin/config/people/accounts
, and set Who can register accounts? toVisitors
, then click Save configuration. (warning: you probably don't want to use this setting on a real site, but it's good enough to test openid_connect_windows_aad's basic functionality in D11) -
Set up a Microsoft Entra ID app configuration as described in this module's documentation →
, i.e.:
- Go to
https://portal.azure.com
and log in if needed. Go to the hamburger menu -> All services. Under Identity, click Microsoft Entra ID - Go to Add -> App registration:
- Name =
openid-connect-windows-aad-drupal11-test
- Supported account types = (whatever makes sense for your use case)
- Redirect URI:
- Select a platform =
Web
- Redirect URI =
https://openid-connect-windows-aad.ddev.site/openid-connect/azure_oidc_d11_test
- Select a platform =
... then click Register.
- Name =
- Go to
https://portal.azure.com
again. Go to the hamburger menu -> All services. Under Identity, click Microsoft Entra ID - In the Microsoft Entra ID sidebar, go to Manage -> App registrations and click openid-connect-windows-aad-drupal11-test
- In the openid-connect-windows-aad-drupal11-test app's sidebar, go to Manage -> Certificates & secrets. In the main area of the page, under Client secrets, click New client secret. Set Description =
testd11
and Expires =90 days (3 months)
. Click Add. Copy the Value. - In Drupal, go to
/admin/config/system/keys/add
, enter:- Key name =
oidc_entra_app_key
- Key type =
Encryption
- Key size =
Other
- Custom key size =
320
- Key provider =
Configuration
- [Key] Base64-encoded = TRUE
- Key value = (paste the client secret you created in the previous step)
- [Value] Base64-encoded = FALSE
... click Save. You see the message The key oidc_entra_app_key has been added.
- Key name =
- In the Azure Portal, in the openid-connect-windows-aad-drupal11-test app's sidebar, click Overview. Under Essentials, copy the Application (client) ID.
- In Drupal, go to
/admin/config/people/openid-connect/add/windows_aad
. You see a Add OpenID Connect client form. Enter:- Name =
azure_oidc_d11_test
- Client ID = (paste the client ID you copied in the previous step)
... don't submit the form yet...
- Name =
- In the Azure Portal, still on the openid-connect-windows-aad-drupal11-test app's Overview page, click Endpoints at the top. An Endpoints sidebar opens:
- Copy OAuth 2.0 authorization endpoint (v2) to a temporary file
- Copy OAuth 2.0 token endpoint (v2) to a temporary file
- In Drupal, on the Add OpenID Connect client form:
- Allowed domains = (the scheme and authority part of the OAuth 2.0 authorization endpoint (v2), e.g.:
https://login.microsoftonline.com
- Authorization endpoint = (paste the OAuth 2.0 authorization endpoint (v2) you copied in the previous step)
- Token endpoint = (paste the OAuth 2.0 token endpoint (v2) you copied in the previous step)
- End session endpoint = (leave blank)
- Map user's AD groups to Drupal roles = (unchecked)
- User info endpoint configuration =
Alternate or no user endpoint
- Alternate UserInfo endpoint = (leave blank)
- Use Graph API otherMails property for email address = (unchecked)
- Update email address in user profile = (unchecked)
- Hide missing email address warning = (unchecked)
- Subject key =
sub
- Check that the Redirect URL matches the Redirect URI you entered when setting up the Entra ID App (e.g.:
https://openid-connect-windows-aad.ddev.site/openid-connect/azure_oidc_d11_test
)
... click Create OpenID Connect client. You see the message OpenID Connect client azure_oidc_d11_test has been added.
- Allowed domains = (the scheme and authority part of the OAuth 2.0 authorization endpoint (v2), e.g.:
- Go to
- In Drupal, Go to
/admin/config/development/performance
click Clear all caches
- Go to
- Test the basic module functions in Drupal 11:
- Log out from the administrator account.
- Go to
/user/login
. You should see a Log in with azure_oidc_d11_test button above the Username and Password fields. - Click the Log in with azure_oidc_d11_test button and authenticate with your Microsoft credentials. You are logged in.
- Make sure there are no errors being logged:
- Log out of the
test_editor
user created earlier; and log in as the administrator again - Go to
/admin/reports/dblog
to ensure our setup/testing did not result in any error messages related to this module.
- Log out of the
+1 to RTBC.
I have code-reviewed and manually tested the code in merge request !11, and I'm satisfied with it.
Here's what I did to manually-test !11:
- Install ddev (I tested on version 1.24.1)
- Clone the module, issue fork, and branch:
git clone --branch '1.x' https://git.drupalcode.org/project/field_protect.git && cd field_protect
- clone the projectgit remote add field_protect-3430524 https://git.drupalcode.org/issue/field_protect-3430524.git && git fetch field_protect-3430524
- add a remote for the issue forkgit checkout -b 'project-update-bot-only' --track field_protect-3430524/'project-update-bot-only'
- switch to the branch for merge request !11
- Set up a test site for the module with the ddev/ddev-drupal-contrib plugin:
ddev config --project-type=drupal --docroot=web --php-version=8.3 --corepack-enable --project-name=field-protect
- create a ddev project for testing the moduleddev add-on get ddev/ddev-drupal-contrib && ddev start && ddev poser && ddev symlink-project
- run the setup steps for the ddev/ddev-drupal-contrib addon- Go to
https://field-protect.ddev.site/core/install.php
in a browser. Confirm that I saw the Drupal 11.0.9 installer. Install the site with the "Standard" install profile. - Go to
/admin/people/create
and create a user with the Content editor role. I gave mine the usernametest_editor
.
- Set up the module for testing:
- Go to
/admin/modules
and install field_protect and its dependencies - Go to
/admin/people/permissions
, grant the Content editor role the following permissions:- Field Protect -> Remember field unlock
... click Save permissions.
- Go to
/admin/structure/types/manage/page/form-display
. Click the gear in to the Title row. Check Field Protect: Protect from accidental changes.. In Field Protect: Message, enterChanging the title may result in the URL alias changing!
, then click Update. You see the warning message You have unsaved changes.. Click Save. You see the status message Your settings have been saved. The Title row now shows Field protected against accidental changes 🔒 and, indented, Changing the title may result in the URL alias changing!.
- Go to
- Test the module's basic functionality:
- Log out of the administrator account and log in as the
test_editor
user you created earlier. - Go to
/node/add/page
. Set Title =Lorem
and Body =Ipsum
. Click Save. You see the status message Basic page Lorem has been created. - Click Edit in the primary tabs. You see an Edit Basic page Lorem form. The Title field is greyed out with a Unlock field link.
- Click the Unlock field link. You see a modal that looks like:
Please confirm before you continue
This field is locked
You can edit this field, but you need to understand the implications. This field is protected with the following message:Changing the title may result in the URL alias changing!
Are you sure you want to proceed?
... you see Cancel and Unlock buttons.
- Click the Cancel button in the modal. The modal disappears, and the field remains locked.
- Click the Unlock field link. You see the same modal from earlier.
- Click the Unlock button in the modal. The title field is now editable again. Set Title =
Dolor
. Click Save. You see the status message Basic page Dolor has been updated.
- Log out of the administrator account and log in as the
mparker17 → changed the visibility of the branch 3431824-automated-drupal-11 to hidden.
I have code-reviewed and manually tested the code in merge request !5, and I'm satisfied with it, so I'm marking it as RTBC.
Here's what I did to test !5:
- Install ddev (I tested on version 1.24.1)
- Clone the module, issue fork, and branch:
git clone --branch '8.x-1.x' https://git.drupalcode.org/project/masquerade_log.gitt && cd masquerade_log
- clone the projectgit remote add masquerade_log-3431824 git@git.drupal.org:issue/masquerade_log-3431824.git && git fetch masquerade_log-3431824
- add a remote for the issue forkgit checkout -b 'project-update-bot-only' --track masquerade_log-3431824/'project-update-bot-only'
- switch to the branch for merge request !5
- Set up a test site for the module with the ddev/ddev-drupal-contrib plugin:
ddev config --project-type=drupal --docroot=web --php-version=8.3 --corepack-enable --project-name=masquerade-log
- create a ddev project for testing the moduleddev add-on get ddev/ddev-drupal-contrib && ddev start && ddev poser && ddev symlink-project
- run the setup steps for the ddev/ddev-drupal-contrib addon- Go to
/core/install.php
in a browser. Confirm that I saw the Drupal 11.0.9 installer. Install the site with the "Standard" install profile. - Go to
/admin/people/create
and create a user with the Content editor role. I gave mine the usernametest_editor
. - Go to
/admin/people/create
and create a user with the Authenticated user role. I gave mine the usernametest_user
.
- Set up the module for testing:
- Go to
https://masquerade-log.ddev.site/admin/modules
and install masquerade_log and its dependencies - Go to
/admin/people/permissions
, grant the Content editor role the following permissions:- Masquerade -> Masquerade as Authenticated user
... click Save permissions.
- Go to
/admin/people/permissions
, grant the Authenticated user role the following permissions:- Toolbar -> Use the toolbar
... click Save permissions.
- Go to
- Test the module's basic functionality:
- Log out of the administrator account, and log in as the
test_editor
user you created earlier. - Go to
/masquerade
- you see a Masquerade form. - In the Masquerade as... box, enter
test_user
and click Switch. - You switch to the new user account (see the toolbar). Note when I tested this, it took me to the
/masquerade
form, and I got an "access denied" because thetest_user
account isn't allowed to masquerade... but this is a problem with the masquerade module, not masquerade_log. - Click
Unmasquerade
in the toolbar to unmasquerade. You see the status message You are no longer masquerading as test_user. - Log out of the test_editor user you created earlier. Log in as the administrator again.
- Go to
/admin/reports/dblog
. You see a list of Recent log messages. You see the following log messages:- Type =
masquerade
; Message =User test_editor stopped masquerading as test_user.
- Type =
masquerade
; Message =User test_editor masqueraded as test_user.
- Type =
- Log out of the administrator account, and log in as the
mparker17 → changed the visibility of the branch project-update-bot-only to active.
mparker17 → changed the visibility of the branch project-update-bot-only to hidden.
I've tested the basic functionality of the module at commit 5901169
from merge request !22 on a local D11 site, and it works for me, so +1 to RTBC from me!
(for the sake of brevity, I'm leaving out my test steps, but let me know if you'd like me to post them)
Okay... when I tried this a second time on a different computer, my test case of the basic module functions worked perfectly. As far as I'm concerned, this means the module is working correctly.
@shashi_shekhar_18oct, if I could trouble you to change the core_version_requirement
line in modules/dropzonejs/media_bulk_upload_dropzonejs.info.yml
from core_version_requirement: '>=10.2'
to core_version_requirement: ^10.2 || ^11
as you did before, then I'll do one last code review/test, then mark this as RTBC.
Thank you very much in advance!
Taking a look at the code in merge request !17, I found the following things that will need to be fixed:
- There's still an instance of the old
core_version_requirement: '>=10.2'
inmodules/dropzonejs/media_bulk_upload_dropzonejs.info.yml
... so I'm marking this as "Needs work".
Running manual tests, I couldn't get !17 to work on D11, so I'm marking this as "Needs work". I'm not yet sure if this is something that I'm doing wrong, or an actual problem with D11 compatibility. I'm going to test again on a different computer to see if my setup on this one is broken.
Here's what I did to test !17...
- Install ddev (I tested on version 1.24.1)
- Clone the module, issue fork, and branch:
git clone https://git.drupalcode.org/project/media_bulk_upload.git && cd media_bulk_upload
- clone the projectgit remote add media_bulk_upload-3431856 git@git.drupal.org:issue/media_bulk_upload-3431856.git && git fetch media_bulk_upload-3431856
- add a remote for the issue forkgit checkout -b '3431856-d11_ready' --track media_bulk_upload-3431856/'3431856-d11_ready'
- switch to the branch for merge request !17
- Set up a test site for the module with the ddev/ddev-drupal-contrib plugin:
ddev config --project-type=drupal --docroot=web --php-version=8.3 --corepack-enable --project-name=media-bulk-upload
- create a ddev project for testing the moduleddev add-on get ddev/ddev-drupal-contrib && ddev start && ddev poser && ddev symlink-project
- run the setup steps for the ddev/ddev-drupal-contrib addonmkdir -p web/libraries/dropzone && curl -L -o web/libraries/dropzone/dropzone.min.js https://unpkg.com/dropzone@5/dist/min/dropzone.min.js
- quickly download and install the latest 5.x version of dropzonejsmkdir -p temp-documents
- create a folder outside the webroot to hold the bulk-uploaded documents before the media entities are fully created- Go to https://media-bulk-upload.ddev.site/core/install.php in a browser. Confirm that I saw the Drupal 11.0.9 installer. Install the site with the "Standard" install profile.
- Go to
/admin/people/create
and create a user with the Content editor role. I gave mine the usernametest_editor
.
- Set up the module for testing:
- Go to
/admin/modules
and install media_bulk_upload and its dependencies - Go to
/admin/modules
and install dropzonejs and its dependencies - Go to
/admin/config/media/media-bulk-config/add
, and enter:- Label =
File
- Media types =
Document
- Form mode =
- None -
- Upload location =
../temp-documents
(I tried various other values for this, see below)
... then click
Save
. You see the status message "Created the File Media Bulk Config." - Label =
- Go to
/admin/people/permissions/module/media_bulk_upload
, grant the Content editor role the following permissions:- dropzonejs -> Dropzone upload files
- Help -> Use help pages
- Media Bulk Upload -> File : Use upload form
- Media Bulk Upload -> Configure Media Upload form
- Go to
- If you don't already have a bunch of files to bulk-upload for testing, you can generate 9 ".doc" files with the following command in sh or bash:
cd ~/Desktop && mkdir test_media_bulk_upload && cd test_media_bulk_upload && for planet in Mercury Venus Earth Mars Jupiter Saturn Uranus Neptune Pluto; do echo $planet > $planet.doc ; done
(if you use some other shell, your mileage may vary) -
Test the basic module functions in Drupal 11:
- Log out of the administrator account, and log in as the
test_editor
Content editor user created earlier. - Go to
/admin/help/media_bulk_upload
. Confirm that I can see a help page. - Go to
/admin/config/media/media-bulk-config
. Confirm that I can see a "Bulk upload media" configuration page. - Click Edit next to the File Media Bulk Config I created earlier. Confirm that I can see a "Edit File" form. Click "Save" without making any changes. Confirm that I can see the status message "Saved the File Media Bulk Config."
- Go to
/media/bulk-upload/file
. Confirm that I can see a "Multiple upload" form. - Try bulk-uploading some of the files I generated earlier.
- Expected behavior: the files were successfully uploaded
- Actual behavior: I got an error,
The file could not be uploaded because the destination "../temp-documents/" is invalid
- I tried various other things for Upload location in the configuration...
/var/www/html/temp-documents
,sites/default/files/temp-documents
, etc. These were reflected in the error message, suggesting they were properly saved; but I got the same error in each case.
- I tried various other things for Upload location in the configuration...
- Log out of the administrator account, and log in as the
- Make sure there are no errors being logged:
- Log out of the
test_editor
user created earlier; and log in as the administrator again - Go to
/admin/reports/dblog
to ensure our setup/testing did not result in any error messages related to this module.
- Log out of the
@shashi_shekhar_18oct, thank you very much!
Quick process note: as a general rule, you shouldn't mark your own changes as RTBC → ("it works on my machine" is a cliché because it happens to all of us, regardless of our experience-level!).
So I'm moving it back to "Needs review" for now... but I have time to code review now, test on my own site, and if everything works, mark it as RTBC!
Thanks again!
mparker17 → changed the visibility of the branch 3431856-automated-drupal-11 to hidden.
mparker17 → changed the visibility of the branch project-update-bot-only to hidden.
There are, apparently, 3 branches, 3 merge requests, and 3 patches associated with this issue. Both !16 and !17 were updated recently when I wrote this comment. I code reviewed both branches.
!17:
- adds a valid-but-unconventional
core_version_requirement: '>=10.2'
, - adds a
.gitlab-ci.yml
(even though that wasn't part of the issue scope), src/Form/MediaBulkUploadForm.php
is missing a newline at the end of the file,MediaTypeManager::getTargetFieldMaxSize()
has a change that looks like:
-return !empty($targetFieldSettings['max_filesize']) ? $targetFieldSettings['max_filesize'] : (string) format_size(Environment::getUploadMaxSize()); +return !empty($targetFieldSettings['max_filesize']) ? $targetFieldSettings['max_filesize'] : (string) DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '10.2.0', fn() => ByteSizeMarkup::create(Environment::getUploadMaxSize()), fn() => format_size(Environment::getUploadMaxSize()));
... in this change, I like the use of
\Drupal\Component\Utility\DeprecationHelper
, but it was missing a fallback value that is present in !16 (i.e.:?? 0
, see below)
!16 is identical to !17, except for the following things:
- !16 has a more-conventional
core_version_requirement
- which notably didn't reflect the change inMediaTypeManager::getTargetFieldMaxSize()
below, - !16 does not add a
.gitlab-ci.yml
(in keeping with the issue scope), src/Form/MediaBulkUploadForm.php
has the newline at the end of the file,MediaTypeManager::getTargetFieldMaxSize()
has a change that looks like:
-return !empty($targetFieldSettings['max_filesize']) ? $targetFieldSettings['max_filesize'] : (string) format_size(Environment::getUploadMaxSize()); +return !empty($targetFieldSettings['max_filesize']) ? $targetFieldSettings['max_filesize'] : (string) ByteSizeMarkup::create(Environment::getUploadMaxSize() ?? 0);
... I like the fallback (
?? 0
), but it is missing the use of\Drupal\Component\Utility\DeprecationHelper
, which would drop support 10.2, as ByteSizeMarkup doesn't exist in 10.2.
My recommendation is that we...
- Hide all the files associated with the ticket to reduce confusion (I will do this)
- Close !13 and !16 (I will do this)
- Change the
core_version_requirement
in !17 to something likecore_version_requirement: ^10.2 || ^11
- Add the newline to the end of
src/Form/MediaBulkUploadForm.php
in !17 - Delete the
.gitlab-ci.yml
in !17 - Add the fallback (i.e.:
?? 0
) to theMediaTypeManager::getTargetFieldMaxSize()
return statement from !16.
... if someone who isn't me could take on the last 4 tasks, then I can test again, and re-mark this as RTBC if it all works... my client wants this module updated to D11 so I have been assigned some time to help out in the next month or so.