Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YALB-1186 and YALB-1210: Upgrade to Drupal 10 and Pin versions in profile composer.json #440

Merged
merged 48 commits into from
Oct 24, 2023

Conversation

codechefmarc
Copy link
Contributor

@codechefmarc codechefmarc commented Sep 27, 2023

YALB-1186: Upgrade to Drupal 10

YALB-1210: Pin versions in profile composer.json

Description of work

  • Upgrades to Drupal 10
  • Updates custom code to be compatible with Drupal 10: ys_views_basic CSS, ys_layouts UpdateExistingNodes.php
  • Adds a replacement fork MR for Search API HTML Element Filter to work with Symfony 6 (see https://www.drupal.org/project/search_api_html_element_filter/issues/3390283)
  • Adds the composer package laminas/laminas-escaper for BugHerd compatibility with Drupal 10
  • Pins versions of contrib modules in profile composer.json file
  • See link to Drupal 10 notes Teams spreadsheet in Jira ticket for specific notes about each contrib module pinning and Drupal 10 compatibility

Specific Module Testing

The following have not been tested as I wasn't aware how to test them. Perhaps someone from Yale is better able to test:

  • Captcha
  • Mailchimp Transactional

General Testing Note

Drupal 10 operates almost entirely like Drupal 9.5, but there were some changes. I did my best to test as much as I could on the multidev but it would be best to get as many eyes on this as possible and test multiple areas of the site. Below are some specific areas that needed some changes in reaction to the upgrade to Drupal 10.

Front-end testing steps:

Back-end testing steps:

  • Visit the multidev as a site admin role
  • Visit the content overview page https://pr-440-yalesites-platform.pantheonsite.io/admin/content
  • Filter content by Tags which uses Chosen and verify that Chosen works as expected and that the filters work as expected.
  • Visit any page on the site and enter layout builder
  • Click "Add block" at the bottom of the content region
  • In the modal to choose a block, start typing "vi" and verify that the list only shows a few blocks that match

Screenshot 2023-10-04 at 4 13 06 PM

  • Add a block of type "View" to the page
  • Save the block and verify that the view shows as expected
  • Configure the view block that was just added and verify that the sidebar for the view is styled correctly

Screenshot 2023-10-04 at 4 19 16 PM

  • Verify that the Chosen interface is styled and works the same as Drupal 9

Screenshot 2023-10-04 at 4 19 23 PM

  • Verify that the rest of the layout builder interface (outside of editing blocks) looks the same as Drupal 9

Screenshot 2023-10-04 at 4 18 53 PM

  • Test other component blocks - verify that adding other component blocks to the page works as expected

…formats, auto entity label, and autosave form
…dcumb, editoria11y, emulsify, entity redirect, fast 404, field group, formdazzle, gin, and gin layout builder
@codechefmarc codechefmarc requested a review from joetower October 4, 2023 23:30
@codechefmarc codechefmarc marked this pull request as ready for review October 4, 2023 23:31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if the permissions on this file were supposed to change or not but figured I would mention it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so I followed these instructions and at the end (step 5) it said to change file permissions for that file to 644.
https://www.drupal.org/docs/upgrading-drupal/upgrading-from-drupal-8-or-later/upgrading-a-composer-based-site
We can certainly change it back to 755 if we want as that is still not world/group writable.

Copy link
Contributor

@vinmassaro vinmassaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good @codechefmarc! I tried to do a drush site install locally on this branch to test installing from scratch and it failed due to not being able to install google_analytics from default config. Luckily there was a patch available so I added it in to resolve the issue.

@codechefmarc
Copy link
Contributor Author

Thanks, @vinmassaro! I'm looking into the smart_date patch to see if we still need it an also the file permission on services.yml as well.

Copy link

@kara-franco kara-franco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work! I went through the public-facing pages and also built out a test page. All behaviors and displays looked similar to D9.

One odd thing I noticed, that resolved after re-setting a setting:
The Pre-built From submit button was failing contrast (pictured below). I went in and re-set the Theme settings for Global Theme and Button Theme and the issue resolved and I cannot for the life of me recreate it. I think it's a non-issue? Maybe a remnant of the palette updates?

Besides that, looks amazing! I'm OOO next week, so if any tweaks or a11y things come up, Mike V can help out!

Screenshot 2023-10-06 at 2 17 26 PM

@miketullo95
Copy link
Contributor

miketullo95 commented Oct 12, 2023

Going through use cases and noticed these two things:

  1. Theme settings accent buttons don't show the colors
    Screen Shot 2023-10-12 at 10 55 41 AM

  2. Mega menu style is off - missing white accents/line under "explore ..."
    EDIT: I'm noticing that the line color is assigned to header color, which was yale blue in this case. So it is showing, its just the same color as the heading. Was this specific instance talked about during the mega nav work a few weeks back? Maybe we could bring this up in standup, asking if this is fine.

Screen Shot 2023-10-12 at 10 57 45 AM

@codechefmarc
Copy link
Contributor Author

@miketullo95 - The header accent colors are now showing up on the multidev, thanks for pointing that one out!

@vinmassaro
Copy link
Contributor

@codechefmarc it occured to me that we are pinning versions in the profile's composer.json, but not in the root composer.json where Drupal core packages are defined. The drupal/core* packages should probably be pinned as well so we have a consistent version of drupal core.

@dblanken-yale
Copy link
Contributor

@codechefmarc Is the recommended way to upgrade to remove ./vendor/* and the lock file and then npm run setup on an existing D9 instance? I ask because I did get it working and it's fine now, but it did take a couple of tries from me to get there. Wanted to make sure I had the upgrade path right.

@codechefmarc
Copy link
Contributor Author

@codechefmarc it occured to me that we are pinning versions in the profile's composer.json, but not in the root composer.json where Drupal core packages are defined. The drupal/core* packages should probably be pinned as well so we have a consistent version of drupal core.

Good call - yes, that should be included here. I don't have hours this week to tackle this, but @nJim may?

@codechefmarc
Copy link
Contributor Author

@codechefmarc Is the recommended way to upgrade to remove ./vendor/* and the lock file and then npm run setup on an existing D9 instance? I ask because I did get it working and it's fine now, but it did take a couple of tries from me to get there. Wanted to make sure I had the upgrade path right.

@dblanken-yale - Yah, locally, I had some small issues with composer so I had to delete the lock file and the vendor directory and then run lando composer install. I was a little worried about that, but, the CI and the multidev worked great, so I think things should be fine. My thought on this is that the local was simply built on the previous 9.5 version and the multidev the artifact is being created from scratch?

Copy link
Contributor

@miketullo95 miketullo95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looked great! Went through all of my AX use cases and only tasks that failed were

  • CAS Restricted Nodes (Known issue, being worked on now)
  • Adding a NetID user (did not have an NetID to test that was not already added to the environment)

@nJim nJim changed the base branch from develop to master October 24, 2023 16:34
@nJim nJim merged commit b61f440 into master Oct 24, 2023
3 checks passed
@vinmassaro vinmassaro deleted the YALB-1186--d10 branch January 9, 2024 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants