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

Wrong url for update.php when domain module working. #39

Closed
izmeez opened this issue Oct 23, 2024 · 12 comments · Fixed by #46
Closed

Wrong url for update.php when domain module working. #39

izmeez opened this issue Oct 23, 2024 · 12 comments · Fixed by #46
Labels
status - fixed This issue is fixed now

Comments

@izmeez
Copy link
Contributor

izmeez commented Oct 23, 2024

With the domain module working, when there is a need to run /core/update.php such as from Backdrop 1.29.0 to 1.29.1 login to the primary domain, going to the Status Report and clicking the link to run update fails because it is trying to reach the wrong url /core/core/update.php.

Disabling the domain module by commenting the line in settings.php file to include domain/services.inc allows the update link to work and uses the correct url.

Subsequently, when the line in settings.php is again uncommented and the site refreshed (clear caches), user has to login again. Something to be expected?

@rudy880719
Copy link
Member

@izmeez Yes, it is normal behavior, since an update was made, and it will ask to log in again

@yorkshire-pudding yorkshire-pudding added type - bug report Something isn't working status - blocker Issue is significant enough that it blocks release labels Oct 24, 2024
@yorkshire-pudding
Copy link
Collaborator

I'm not worried about the logging in again, but it does seem like a bug that it goes to the wrong URL and I think this is a problem as it effectively makes it impossible to do updates completely in the UI, which is a key feature of Backdrop.

@izmeez
Copy link
Contributor Author

izmeez commented Oct 24, 2024

Agree: logging in again may be an artifact of changing the settings.php file; the key problem is the wrong url for update.

@sudipto68
Copy link
Contributor

sudipto68 commented Nov 8, 2024

@izmeez I can also reproduce this issue. But in the module's upgrade.txt file and also in the install_quickstart file it is written that when we need to update the site we need to remove that line from the settings.php file.

When upgrading, follow the additional steps below to ensure that the required
changes for Domain Access do not interfere with Drupal's update process.

-- Remove the Domain Access include line from settings.php.

@yorkshire-pudding
Copy link
Collaborator

@sudipto68 @izmeez @stpaultim @rudy880719
I think it is important that we try to overcome this for Backdrop. Breaking the update process to me seems like a real problem for a contrib module.

Perhaps we need wider support from people in the community with more expertise on this one?

elisseck added a commit to elisseck/domain that referenced this issue Nov 11, 2024
does it, accounting for "/core" scripts to auto-detect a base
path.

Resolves backdrop-contrib#39
@izmeez izmeez added pr - works for me I tested the Pull Request, it works for me status - has pull request This issue has a Pull Request pr - needs code review The Pull Request needs to be code-reviewed labels Nov 11, 2024
@izmeez
Copy link
Contributor Author

izmeez commented Nov 11, 2024

@elisseck Thank you for the PR. I have tested it and it works for me. I have updated the labels for the issue.

@izmeez
Copy link
Contributor Author

izmeez commented Nov 11, 2024

@elisseck The PR also appears to fix #41

@sudipto68
Copy link
Contributor

@elisseck Thank you for the PR. I have tested it and it works for me too. After adding the changes, If I run the core/update.php path to manually run the update, then it works perfectly and now no need to remove the include bootstrap line from settings.php file.

@stpaultim
Copy link
Member

Thanks to @izmeez and @sudipto68 for testing. I went ahead and merged this PR and created a beta release of the module to encourage additional testing.

@stpaultim
Copy link
Member

And special thanks to @elisseck for submitting the PR.

@yorkshire-pudding
Copy link
Collaborator

Thanks @elisseck @stpaultim @izmeez @sudipto68

@stpaultim stpaultim added status - fixed This issue is fixed now and removed pr - needs code review The Pull Request needs to be code-reviewed status - blocker Issue is significant enough that it blocks release pr - works for me I tested the Pull Request, it works for me status - has pull request This issue has a Pull Request labels Nov 12, 2024
@izmeez
Copy link
Contributor Author

izmeez commented Nov 12, 2024

This PR also appears to fix issue #36.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status - fixed This issue is fixed now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants