We have a problem - InstallerController::activate() is capable of activating multiple projects in one shot, but it can only return feedback for one of them (the first one that returns a response).

The problem is that activators can return either nothing, or a complete response. That's too strong of an opinion. It also means we can never return an updated Project object to the frontend, with its post-activation information added to it, from the activate() method if an activator returns a response.

What we should do here is use the good ol' Drupal AJAX system for activation only. Activators should no longer be able to return full responses, but instead either an array of AJAX commands to add to a fuller AjaxResponse, or null (to add nothing to the response). That response should also include AJAX commands that wrap around the updated Project objects, so that those can be delivered coherently to the frontend.

As part of this change, we should split out ActivationManager::getActivationInfo() method into a dedicated normalizer that handles Project and ProjectsResultPage objects and turns them into data that can be consumed by the frontend. This would be a cleaner design for a few reasons:

  • The job that getActivationInfo() is actually doing is normalization, so we might as well formally recognize that by moving it into a normalizer.
  • Normalizers have an interface, which means that modules that wish to extend Project Browser could add more activation-related superpowers.
  • Modules that want to swap out Project Browser's frontend entirely (which is an unlikely, but theoretically legitimate possibility) would be able to normalize Project objects in ways that make sense to them.
CommentFileSizeAuthor
#10 errors.png321.24 KBchrisfromredfin

Issue fork project_browser-3506454

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

phenaproxima created an issue. See original summary.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue summary: View changes
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue summary: View changes

phenaproxima opened merge request !738

narendrar made their first commit to this issue’s fork.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Title: Turn ActivationManager::getActivationInfo() into a dedicated normalizer » Turn ActivationManager::getActivationInfo() into a dedicated normalizer, and make activation use the Drupal AJAX system
Status: Active » Needs review
Issue tags: +Needs issue summary update
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue summary: View changes
Issue tags: -Needs issue summary update

tim.plunkett made their first commit to this issue’s fork.

chrisfromredfin made their first commit to this issue’s fork.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Needs review » Needs work
StatusFileSize
new errors.png321.24 KB

having an issue installing the RDF module when max_selections == 1 

failed request to https://pb11.ddev.site/admin/modules/project_browser/install-require/vJGZ7DQb3s7smsKB7g8ugr3MADpHwq9C: {"message":"StageException: Failed to run process: \u003Cem class=\u0022placeholder\u0022\u003EThe command \u0026quot;\u0026#039;\/var\/www\/html\/vendor\/bin\/composer\u0026#039; \u0026#039;--working-dir=\/tmp\/.package_managerf67b6eaa-503f-4bec-b2fa-795a3972b530\/vJGZ7DQb3s7smsKB7g8ugr3MADpHwq9C\u0026#039; \u0026#039;update\u0026#039; \u0026#039;--with-all-dependencies\u0026#039; \u0026#039;--optimize-autoloader\u0026#039; \u0026#039;drupal\/rdf\u0026#039;\u0026quot; failed.\n\nExit Code: 2(Misuse of shell builtins)\n\nWorking directory: \/var\/www\/html\/web\n\nOutput:\n================\n\n\nError Output:\n================\nLoading composer repositories with package information\nUpdating dependencies\nYour requirements could not be resolved to an installable set of packages.\n\n  Problem 1\n    - Root composer.json requires drupal\/rdf ^2.1 -\u0026gt; satisfiable by drupal\/rdf[2.1.0, 2.1.1, 2.x-dev].\n    - drupal\/rdf[2.1.0, ..., 2.x-dev] require drupal\/core ^9.4 || ^10.0 -\u0026gt; found drupal\/core[9.4.0-alpha1, ..., 9.5.x-dev, 10.0.0-alpha1, ..., 10.5.x-dev] but these were not loaded, likely because it conflicts with another require.\n\n\u003C\/em\u003E","phase":"require"} Response {type: 'basic', url: 'https://pb11.ddev.site/admin/modules/project_brows…/install-require/vJGZ7DQb3s7smsKB7g8ugr3MADpHwq9C', redirected: false, status: 500, ok: false, …}

popup.js:56 Uncaught (in promise) TypeError: Drupal$1.dialog is not a function
    at openPopup (popup.js:56:29)
    at handleError (InstallListProcessor.js:87:3)
    at doRequests (InstallListProcessor.js:181:15)
    at async processInstallList (InstallListProcessor.js:223:5)
    at async HTMLButtonElement.onClick (ActionButton.svelte:47:42)
chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Needs work » Reviewed & tested by the community

j/k this didn't happen after a reinstall. I think my local was just a little messed up.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Reviewed & tested by the community » Fixed

this is way cool. two follow-ups adam is creating that are UI-related bugs when there's an error, but not directly caused by this code and those will be small, so prefer them in FUs.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.