feat: Decouple all 3 themes to Drupal.org

podarok commented 2 years ago

Original Issue, this PR is going to fix: https://openy.atlassian.net/browse/MAINTAIN-144

Make sure these boxes are checked before asking for review of your pull request - thank you!

If there is a new feature or this is a bug fix - use 9.x-2.x branch. We'll tag for release if the bug is critical asap or tag for release next bug fix release until critical issue arrived.

Steps for review

[ ] Please provide steps for review here.
[ ] Please provide steps for review here.

General checks

[ ] All coding styles are fulfilled and there are no any issues reported by CodeSniffer. See Code of Conduct.
[ ] Try to use Conventional commit messages accordingly to the standard https://www.conventionalcommits.org/en/v1.0.0/#specification
[ ] Documentation has been updated according to PR changes.
[ ] Steps for review have been provided according to PR changes.
[ ] Make sure you've provided all necessary hook_update_N to support upgrade path.
[ ] Make sure your git email is associated with account on drupal.org, otherwise you won't get commits there.
[ ] If you would like to get credits on drupal.org, check documentation.

Thank you for your contribution!

gundevel commented 2 years ago

Build comment file:

Check Open Y Installation Wizard at http://install.openy.cibox.tools/build3622/install.php

Test PASSed. Refer to this link for build results (access rights to CI server needed): http://openy.cibox.tools:8080/job/PR_BUILDER_COMPOSER/3622/

gundevel commented 2 years ago

Open Y Upgrade Path site check installed at http://upgrade.openy.cibox.tools/build3622/

podarok commented 2 years ago

froboy commented 2 years ago

@podarok I'm not able to recreate this issue. I've required "ymcatwincities/openy": "dev-dev_themes_decouple", and I'm building my local with a full fresh install using fin init. Is there another process you're using to get to this point?

podarok commented 2 years ago

@froboy yes, I found this on upgrade path. you should install previous version and apply updated branch on top

podarok commented 2 years ago

But good to know profile install not affected

froboy commented 2 years ago

@podarok I'm still not able to recreate the issue. Here's what I did this time around:

# Set up project with composer create-project and docksal
fin composer require ymcatwincities/openy:"^9.2" -W
fin init
# visit /locations and observe it works 
fin composer require ymcatwincities/openy:dev-dev_themes_decouple -W
fin drush updb # no updates necessary
fin drush cr

I'm running into some other errors like this, but most of the time disabling and reenabling the module resolves it.

Error: Class 'Drupal\openy_programs_search\DataStorage' not found in Drupal\Component\DependencyInjection\Container->createService() (line 262 of /var/www/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php)

I was able to resolve that issue with: fin drush pmu openy_programs_search && fin drush en openy_programs_search -y && fin drush cr

Could you please send a db from your failing instance or provide more detailed repro steps. Thanks.

froboy commented 2 years ago

@podarok I logged into the cibox version above and was able to figure out the issue. At admin/openy/settings/openy_map the "Locations Map Icon" fields were not set. This somehow results in the map... breaking badly. Once I filled in the field for each section and saved the form the map displays properly. I've seen this issue on the sandbox before so i think it might be something in the install scripts. I'd recommend we split the issue off separately though as it seems like it's not related to the theme decoupling.

podarok commented 2 years ago

retest this please

gundevel commented 2 years ago

Build comment file:

Check Open Y Installation Wizard at http://install.openy.cibox.tools/build3629/install.php

Test PASSed. Refer to this link for build results (access rights to CI server needed): http://openy.cibox.tools:8080/job/PR_BUILDER_COMPOSER/3629/

gundevel commented 2 years ago

Open Y Upgrade Path site check installed at http://upgrade.openy.cibox.tools/build3629/

podarok commented 2 years ago

Open Y Upgrade Path site check installed at http://upgrade.openy.cibox.tools/build3629/

Check @froboy - it's a newer DB dump for upgrade path ( current stable Open Y ) and the map is working, pins are not

gundevel commented 2 years ago

Build comment file:

Check Open Y Installation Wizard at http://install.openy.cibox.tools/build3630/install.php

Test PASSed. Refer to this link for build results (access rights to CI server needed): http://openy.cibox.tools:8080/job/PR_BUILDER_COMPOSER/3630/

gundevel commented 2 years ago

Open Y Upgrade Path site check installed at http://upgrade.openy.cibox.tools/build3630/

ymcatwincities / openy