Problem/Motivation

Recipes are meant to be idempotent and can be re-applied at any time. However, Project Browser currently doesn't allow a second installation of something that's Installed.

Steps to reproduce

Install a recipe from the recipes tab (drupal 10.3+). Note that you cannot re-apply.

Proposed resolution

RecipeActivator should be able to return a "Reapply" task, which uses AJAX to re-invoke the activation route.

CommentFileSizeAuthor
#14 Recipes_reinstall.mov7.7 MButkarsh_33

Issue fork project_browser-3489729

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

chrisfromredfin created an issue. See original summary.

pameeela’s picture

pameeela commented
Issue tags: -Starshot blocker +Drupal CMS stable release blocker

Swapping over to the Drupal CMS tag, I only just realised we are using both.

I'm not 100% sure this is a *blocker*, more of a goal, but happy to leave the tag for encouragement :)

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue tags: -beta blocker, -Drupal CMS stable release blocker

I don't really think this is a stable release blocker. It's pretty easy to implement, so we should get it done, but it need not be a stable blocker. Or even a beta blocker for Project Browser.

phenaproxima’s picture

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

pameeela commented
Issue tags: +Drupal CMS release target

Adding target tag so we can track these nice to haves.

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

utkarsh_33 opened merge request !642

utkarsh_33’s picture

utkarsh_33 commented
Status: Active » Needs review

So currently if we apply a recipe the status card shows installed but as soon as we refresh the page the select button appears again.I am not sure if that's the desired behaviour or we want something like once the recipe is installed a re-install button should appear appear on the card(which i think is a better approach and a more user friendly approach).
Marking it NR to get others opinions on this.

pameeela’s picture

pameeela commented

as soon as we refresh the page the select button appears again

I've never seen this behaviour, for me, it shows as 'Installed' always after it is applied.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Needs work

Looks like this needs to be re-synced with HEAD.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Oh, we can also completely remove the event subscriber stuff from RecipeActivator since we're no longer tracking it.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Although...hmmm. Do we want to remove the tracking? It's true we're not using it anywhere, but it might be nice to show folks that they already applied a recipe. That would take some UX work, though, and the current way PB works dictates that if it's installed, it's not re-installable.

So maybe the right approach here is to leave the tracking as-is and work out the proper UX (in a follow-up) to show some kind of "already applied" badge to recipes that have been applied.

utkarsh_33’s picture

utkarsh_33 commented

RE #9:- I still see what i mentioned in #8.I'll try to record a video and upload.In the mean time if anyone else can also verify the current state.

utkarsh_33’s picture

utkarsh_33 commented
StatusFileSize
new Recipes_reinstall.mov7.7 MB

RE #9 Please see the recording.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented

On the latest 2.0.x as of right now (46ab2a2) - I can go to recipes tab, apply a recipe, it shows "installed ✔️" - then I reload the page, and it still says installed.

In an IDEAL world, it would be nice to actually record a LOG of when recipes were applied and be able to show the user that it has already been applied at such and such time/date. However, I'm not yet sure what the UI for that would be.

I THINK my personal belief is that given the option of what we have now (disallowing reinstall) or allowing users to re-apply (and they simply must track themselves if they've applied a recipe before or not), I THINK I would choose the latter. But I'm happy to be outvoted by other UX folks' opinions.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Test cases that need to happen here:

  1. A recipe with no input should trigger a batch job and redirect back to the page we were on.
  2. A recipe with input should show a form, and once that form is submitted, it should trigger a batch job and redirect back to the page we were on. If there are validation errors, the form should always refresh until those are resolved.
  3. Trying to apply a recipe that isn't installed should set an error and redirect back to the page we were on.
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Title: Allow recipes to be re-"installed" (re-applied) inside PB » Allow recipes to be re-applied from the Project Browser UI

phenaproxima changed the visibility of the branch 3489729-allow-recipes-to to hidden.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Wow, this one is turning out to be really effing hard to pull off.

I'm going to back off and try to think of an approach that will actually work.

phenaproxima opened merge request !779

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
Issue tags: +Needs tests

Needs tests, but I found an approach that makes this work without major UI behavior changes and extensive refactoring.

Long story short: allow follow-up tasks to take advantage of the use-ajax class.

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

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue tags: -Needs tests

Test written - I think we're good here.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented

Initial manual testing here seems to imply this is working well. Small pieces of feedback:

(1) the dropbutton with only one action in it looks janky. Not sure if we leave the carat, or shrink the space, or what. But something.

(2) having an issue with 'Drupal2.$dialog is not defined' or something similar. I have seen this in other MRs so I don't think it's related to this issue. But I've seen it enough that it should have a FU.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Related issues: +#3512842: The drop button doesn't look right when there is only one follow-up task, +#3512846: Weird JavaScript errors with aggregation enabled in some situations
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Related issues: +#3512856: Create a helper function to handle project ID qualification
phenaproxima’s picture

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

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

Saving credit

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Reviewed & tested by the community » Fixed

Merged !779 🎉

Status: Fixed » Closed (fixed)

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