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

Update Command - Multisite shared modules are replaced by site specific #299

Open
Tracked by #111
yorkshire-pudding opened this issue Jun 27, 2023 · 5 comments · May be fixed by #347
Open
Tracked by #111

Update Command - Multisite shared modules are replaced by site specific #299

yorkshire-pudding opened this issue Jun 27, 2023 · 5 comments · May be fixed by #347
Labels
feature-branch Relates to functionality in a feature branch not in 1.x-1.x type: bug

Comments

@yorkshire-pudding
Copy link
Collaborator

This is a subtask of #111 and relates to the issue-111-update-command branch.

Steps to reproduce

  1. Download an old version of a module to the shared module folder
  2. Run bee --site=site_name update

Expected result
Module is updated in current location

Actual result
Module is deleted from shared module folder and installed in the site specific module folder.

The cause of this is using the download_bee_callback which when invoked with the site option will download to the site folder.

I think we should fix this at the same time as #297

@yorkshire-pudding yorkshire-pudding added type: bug feature-branch Relates to functionality in a feature branch not in 1.x-1.x labels Jun 27, 2023
@yorkshire-pudding yorkshire-pudding changed the title Update Command: Multisite shared modules are replaced by site specific Update Command - Multisite shared modules are replaced by site specific Jun 27, 2023
@hosef
Copy link

hosef commented Nov 22, 2023

@yorkshire-pudding I think it would be a good idea to allow updating a module in just one site even if the module already exists in the shared modules folder. That way you can specify a different version if one site requires it.

We should still stop it from deleting the module from the shared modules folder.

@yorkshire-pudding
Copy link
Collaborator Author

Thanks @hosef - yes I could see that being very useful for multi site owners.

hosef added a commit to hosef/bee that referenced this issue Nov 22, 2023
@hosef hosef linked a pull request Nov 22, 2023 that will close this issue
@yorkshire-pudding
Copy link
Collaborator Author

yorkshire-pudding commented Nov 23, 2023

@hosef - I don't think this PR is working as it probably needs to. I think to meet the scenario above there should probably be an option for the update command to update a site only as I think the default should be to update the shared.

I tried to do it how I might normally do it in the test install.

  1. Added bee_message($folder, 'log'); to line 177 of update.bee.inc
  2. Manually installed older version within the UI (I'm going to try to do some more work on Allow specifying a version for the pm-download command #56 to make it easier to test). On multisite, installing with the UI automatically puts it in the shared folder. Normally, in bee, we can do bee dl menu_pager and it will install to the shared folder as download doesn't use bootstrap.
  3. Ran lando bee --site=multi_one -d update (note bee update will not work without a site specified as it can't bootstrap to get update data)

Expected result
Updates the shared version

Actual result

These are the items being updated:

| Module     | Existing  | Latest    |
| menu_pager | 1.x-1.3.1 | 1.x-1.4.2 |

Would you like to continue? (y/N): y

 [-] 'Debug' mode enabled.
 [-] /app/multisite/sites/multi_one/modules/menu_pager
 [-] GitHub API Rate Limit: 25/60 used with 35 remaining. Quota will reset at 11:11 UTC.
 [-] Core projects are: admin_bar,block,book,ckeditor,color,comment,config,contact,contextual,dashboard,date,dblog,email,entity,entityreference,field,field_ui,file,filter,image,installer,language,layout,link,locale,menu,node,path,redirect,search,simpletes     t,syslog,system,taxonomy,telemetry,translation,update,user,views,views_ui,bartik,basis,engines,seven,stark,boxton,geary,harris,legacy,moscone,moscone_flipped,rolph,simmons,sutro,taylor,taylor_flipped
 [-] GitHub API Rate Limit: 25/60 used with 35 remaining. Quota will reset at 11:11 UTC.
 [-] GitHub API Rate Limit: 26/60 used with 34 remaining. Quota will reset at 11:11 UTC.
 [✘] 'menu_pager' already exists in '/app/multisite/modules/menu_pager'.

Perhaps something like this:

Scenario - module exists in shared and does not exist in site
bee --site=multi_one update --multisite-option=site-only
Action
Bee leaves shared module alone
Bee downloads latest version to site module folder.

Scenario - module exists in shared and does not exist in site
bee --site=multi_one update
Action
Bee updates shared module folder to latest version .

Scenario - module exists in shared and also exists in site module folder
bee --site=multi_one update --multisite-option=site-only
Action
Bee leaves shared module alone
Bee updates site module folder to latest version .

Scenario - module exists in shared and also exists in site module folder
bee --site=multi_one update --multisite-option=shared-only
Action
Bee leaves site module alone
Bee updates shared module folder to latest version .

Scenario - module exists in shared and also exists in site module folder
bee --site=multi_one update --multisite-option=site-only
Action
Bee leaves shared module alone
Bee updates site module folder to latest version .

Scenario - module exists in shared and does not exist in site
bee --site=multi_one update --multisite-option=site-only
Action
Bee leaves shared module alone
Bee downloads latest version to site module folder.

Scenario - module exists in site and does not exist in shared
bee --site=multi_one update
Action
Bee updates site module folder to latest version.

As an aside, I think we need to separate the changes to download.bee.inc and do those first against 1.x-1.x - I'll then merge back to this branch before we do the change to update. This is to reduce the risks of conflicts when I'm ready to merge this feature branch into the 1.x-1.x branch. However, I'm happy to keep them in this PR for now for ease of testing.

@hosef
Copy link

hosef commented Nov 23, 2023

Ok, I tried to convert the scenarios into a more tabular form so it's easier to scan.

Command                                      Shared Site Effect
bee up                                       No     No   Do nothing
                                             Yes    No   Update shared folder
                                             No     Yes  Do nothing
                                             Yes    Yes  Update shared folder

bee up --site=<site>                         No     No   Do nothing
                                             Yes    No   Update shared folder
                                             No     Yes  Update site folder
                                             Yes    Yes  ???

bee up --site=<site> --multisite=shared-only No     No   Do nothing
                                             Yes    No   Update shared folder
                                             No     Yes  Do nothing
                                             Yes    Yes  Update shared folder

bee up --site=<site> --multisite=site-only   No     No   Do nothing
                                             Yes    No   Do nothing
                                             No     Yes  Update site folder
                                             Yes    Yes  Update site folder

All of those make sense, but there is one that was not addressed. What should happen when it exists in both locations and the multisite flag is not specified?

As for the download command, I think that it would be a good idea to make a more generic download function that has options passed into it instead of accessing global variables or command line arguments directly. Off the top of my head, I think it would need: project name, download destination, version, if it should replace an existing version, if it should make a backup of the project. While we are at it, we should have other functions that handle getting paths and handling version info. We could then make all the commands that need to download projects wrap these other functions so that they behave consistently.

@yorkshire-pudding
Copy link
Collaborator Author

All of those make sense, but there is one that was not addressed. What should happen when it exists in both locations and the multisite flag is not specified?

If the module exists in both locations then the one that is active is the 'site' one on that site, so one argument could be to just to that. Another approach, as we haven't specified an option is to update both.

I think the first approach is perhaps more what people would expect.

As for the download command, I think that it would be a good idea to make a more generic download function that has options passed into it instead of accessing global variables or command line arguments directly. Off the top of my head, I think it would need: project name, download destination, version, if it should replace an existing version, if it should make a backup of the project. While we are at it, we should have other functions that handle getting paths and handling version info. We could then make all the commands that need to download projects wrap these other functions so that they behave consistently.

I've created a separate issue for that: #348

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-branch Relates to functionality in a feature branch not in 1.x-1.x type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants