Skip Package Manager validation of using the local recipes source plugin

Created on 17 December 2024, 4 months ago

Problem/Motivation

Right now, if Package Manager is available and UI install is enabled, Project Browser will always validate the current system state on every Project Browser page. This is both wasteful and too much of a sledgehammer; for example, when viewing local recipes, you don't need Package Manager at all.

Proposed resolution

This is a really dirty hack, but for now, in \Drupal\project_browser\Element\ProjectBrowser::getDrupalSettings(), we should skip doing the Package Manager install readiness checks if $source === 'recipes'.

In another issue, we should do a more detailed and permanent fix, which is that the readiness checks should only ever be done for projects whose activation status is ActivationStatus::Absent. Package Manager is not relevant in any other circumstance.

📌 Task
Status

Active

Version

2.0

Component

Code

Created by

🇺🇸United States phenaproxima Massachusetts

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @phenaproxima
  • First commit to issue fork.
  • Pipeline finished with Failed
    4 months ago
    Total: 574s
    #373397
  • 🇮🇳India shalini_jha

    I have attempted to add a check to skip the Package Manager install readiness checks specifically for recipes. However, I am unsure if this is causing the Nightwatch failure

  • 🇮🇳India shalini_jha

    I believe this failure is not related to the changes I made. I am moving this for review. Please take a look and let me know if this aligns with the expectations of the ticket or if any updates are required.

  • 🇺🇸United States phenaproxima Massachusetts

    The Nightwatch failures are preexisting; not your fault.

    I think that doing a skip if the source is recipes is a completely legitimate workaround.

    I kind of wonder, though, if it would be super hard to move the install readiness checks to \Drupal\project_browser\Controller\InstallerController::begin(). Because that is the only time Package Manager will ever be invoked. The only thing the Svelte app would need to know is whether Package Manager is present at all. It wouldn't have to care about the validation results or anything like that.

    Maybe we can try the deeper fix in another branch of this issue fork, and then commit the quick fix if it turns out to be a bugaboo? Or, hell, we could commit the quick fix but keep this issue open and work on the deeper fix in another branch.

    In either case, though, this branch needs to be synced against 2.0.x HEAD due to merge conflicts. :(

  • 🇮🇳India shalini_jha

    @phenaproxima Thank you for the review & feedback. i am fixing this conflict issue till then.

  • Pipeline finished with Failed
    4 months ago
    Total: 627s
    #376873
  • 🇮🇳India shalini_jha

    Fixed merge conflict , as mentioned in #6 not moving this ticket.

  • First commit to issue fork.
  • Pipeline finished with Failed
    3 months ago
    Total: 513s
    #386980
  • 🇮🇳India narendraR Jaipur, India

    Re #6, The problem with moving the install readiness checks to \Drupal\project_browser\Controller\InstallerController::begin() is:

    • The user will not know beforehand if there are errors or warnings on the page.
    • sveltejs/src/Project/ActionButton.svelte uses this value to work/display the button based on errors or warnings.
  • 🇺🇸United States phenaproxima Massachusetts

    Thanks for looking into that, @narendrar. If that's the case, let's proceed with the quick-fix for now but open a follow-up (linked in a comment above the fix) to improve this.

  • 🇮🇳India narendraR Jaipur, India
  • 🇺🇸United States phenaproxima Massachusetts

    A fine workaround, in my opinion; ship it.

  • Pipeline finished with Failed
    3 months ago
    Total: 753s
    #387370
  • First commit to issue fork.
  • Pipeline finished with Skipped
    3 months ago
    #387705
  • 🇺🇸United States chrisfromredfin Portland, Maine

    Thanks Naren for bringing up those points, which are important. So, this is a good workaround for now in order to get Drupal CMS out the door.

  • Pipeline finished with Failed
    3 months ago
    Total: 635s
    #387702
  • Automatically closed - issue fixed for 2 weeks with no activity.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 1191s
    #422157
Production build 0.71.5 2024