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

Fix #10375 - Upgradewizard double commit #10375

Conversation

JanSiero
Copy link
Contributor

This PR fixes an issue that "commit" is called twice when runing the upgradewizard caused by changes in:
890e599#diff-7c5c0ada659df5cc72d508d4b597f1882ede2ee19a4a243010a84b9a3a8b4213

Description

The changes in the said commit introduce the variable $additionalStep in modules/Upgradewizard/index.php
This variable is initalized in line 509, but already used before in line 297 without initialization. This causes the commit.php to be executed twice.

Motivation and Context

As commit.php is secured to not run its logic twice, this bug does not have any implications for now, however future changes might cause an issue here.

How To Test This

  • install SuiteCRM 7.14.2
  • upgrade to SuiteCRM 7.14.3

Expected result
commit.php is only called once

Observed behaviour
the file upgradewizard.log shows two entries of the line "[At commit.php]"
This can also be verified by debugger.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Final checklist

  • My code follows the code style of this project found here.
  • My change requires a change to the documentation.
  • I have read the How to Contribute guidelines.

@JanSiero JanSiero force-pushed the bugfix_upgradewizard_double_commit branch from e7d40a4 to 5aa0e23 Compare March 10, 2024 03:41
@JanSiero JanSiero changed the title Fix # - Upgradewizard double commit Fix #10375 - Upgradewizard double commit Mar 10, 2024
@serhiisamko091184 serhiisamko091184 added Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member Area: Clean Up: Performance Issues & PRs related to all things regarding to performance Branch:Hotfix PR 4-8 Score given to PRs once assessed Status: Requires Code Review Needs the core team to code review labels Mar 19, 2024
@serhiisamko091184
Copy link
Contributor

Hello @JanSiero,

Thanks for your contribution!

Regards,
Serhii

@jack7anderson7 jack7anderson7 added Status: Requires Testing Requires Manual Testing Status: Passed Code Review Mark issue has passed code review reviewed and removed Status: Requires Code Review Needs the core team to code review labels May 2, 2024
@johnM2401 johnM2401 added Status: Passed Testing and removed Status: Requires Testing Requires Manual Testing labels May 3, 2024
@jack7anderson7 jack7anderson7 merged commit 99219c1 into salesagility:hotfix May 14, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Clean Up: Performance Issues & PRs related to all things regarding to performance Branch:Hotfix PR 4-8 Score given to PRs once assessed Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member Status: Passed Code Review Mark issue has passed code review reviewed Status: Passed Testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants