Upgrading code review

robertovasquez wrote on Tuesday, May 06, 2014:

Hi Forum, the sql_upgrade.php manually requests the version to upgrade from. I had modified the code in the following way: The manually insert of the openemr version has being removed and added the code to select the version form the version table.
Could you please someone take a look of the changes I made and give me your technical opinion.This is the link to the sql_upgrade branch at github

bradymiller wrote on Sunday, May 11, 2014:

Hi,

We discussed this project on the conference call today and was hoping to get some input from the community:

Would it be helpful if the sql_upgrade.php script automatically calculated the version to upgrade from rather than having the user manually enter it. My thoughts were that it would by useful, but I wanted to confirm this.

thanks,
-brady
OpenEMR

bradymiller wrote on Sunday, May 11, 2014:

btw, here’s a link to this project on the Active Projects wiki:
http://www.open-emr.org/wiki/index.php/Active_Projects#Upgrading

blankev wrote on Sunday, May 11, 2014:

IMHO I would be in favour also for the Patch release number. But with the option to manually insert your own name and version etc.

sunsetsystems wrote on Sunday, May 11, 2014:

Right, an option for manual override may sometimes be needed.

Rod
http://www.sunsetsystems.com/

bradymiller wrote on Tuesday, May 13, 2014:

Hi,

So it appears, the script needs to be able to do the following:

1.When a sql “version” table does not exist, then continue the current behavior (ask the user for the version).

2.When a sql “version” table does exist, then calculate the version and run the upgrade script without any user input. There will be two situations in this case:
scenario 1: More common scenario where it is a production version(no “-dev” in the version table in the v_tag column). In this case, can just use the version.
scenario 2: Less common scenario where it is a development version (with “-dev” in the v_tag column). In this case, will need calculate the actual version by integrating the version in the version table with the upgrade files in sql/ (you will need to go to one version below what is in the version table; note you can do this quickly by using the $version array which is already being created in the current sql_upgrade.php script at line 37).

3.Provide a mechanism for user to demand a version, which can already be done when calling the script with a set form_old_version get parameter (and can support this by just changing the if from !empty($_POST[‘form_submit’]) to !empty($_POST[‘form_old_version’])).

4.(This is something I’ll gladly do when above is done). Will add a mechanism to allow running the script via commandline php with a site parameter and a old version parameter, which will simplify the command line scripts that run this upgrade script (this is done in the ubuntu openemr package and the demo farm script).

-brady
OpenEMR

robertovasquez wrote on Sunday, May 25, 2014:

Hi
During Requirements 1, 2 Unit Testing I found other escenario:
When the user has -dev or not -dev and the most updated version(4.1.3). The foreach ($versions as $version => $filename) won’t find the file 4_1_3-to-4_1_4_upgrade.sql in /sql; because it is not there.
Do I need to go back to the most updated version in the $version array or tell the user he/she already has the most updated version-thanks-bie?

Requirement 3:
I understant:
If table version exist, give the user the option to use the select name=‘form_old_version’ and select the select the prior release you are converting from:
This will overwrite requirement 2 (version table exist )
Please advice

You will find the code that meet requirements 1 and 2 in this link:

Thanks

Roberto

bradymiller wrote on Tuesday, May 27, 2014:

Hi Roberto,

If version is up to date, then recommend doing following:
echo to screen that It appears the version is up to date
and also allow the user to select a version to upgrade from.

Also, if the user has something other than -dev or <blank> (some users appear to put stuff here), then also allow user to select a version to upgrade from (since it will be impossible to discern whether it is a dev or production version in this case).

Requirement 3 is a mechanism to provide a version_from via a get parameter on the web url (and can be overrided like this in any situation).

I haven’t looked at the code yet, but recommend placing your code on most recent development version (appears to be on a very outdated version now).

-brady
OpenEMR

robertovasquez wrote on Wednesday, June 04, 2014:

Hi Brady
The sql_upgrade.php Upgrading does the following:

  1. When table version does no exist:
    1.2 It appers no version table exist
    1.3 Please select the prior release you are converting from the select list. Default-> most updated version in sql directory.
    1.4 Or Type here if you need to manual override : Text Box
  2. When no -dev:
    2.1 Your Openemr database version is : 4.1.3
    2.2 v_tag in version table is : prod
    2.3 It appears the version is up to date
    2.4 Please select the prior release you are converting from: selec list. Default-> most updated version in sql directory.
    2.5 Or Type here if you need to manual override : text Box
  3. When -dev:
    3.1 Your Openemr database version is : 4.1.3
    3.2 v_tag table is : -dev
    3.3 Openemr prior release has being selected from the version table: 4.1.2
    3.4 Or if you prefer to select the release you are converting from: sel list. Default-> the previous version.
    3.5 Or Type here if you need to manual override : text Box
    4 The select name=‘form_old_version’ has being modified:
    //if ($version === ‘4.1.2’) echo " selected";
    if ($version === $most_recent_version) echo " selected";
    This code will get the most updated version file on sql directory when a new file will be added.

The code is here :

Roberto

bradymiller wrote on Sunday, June 15, 2014:

Hi Roberto,
Can you provide a link to your github branch. Then we can direct you how to rebase all your work into a new branch with one commit to allow easier review and testing.
thanks,
-brady
OpenEMR

robertovasquez wrote on Sunday, June 15, 2014:

Hi Brady
This is my github my_sql_upgrade branch

bradymiller wrote on Sunday, June 15, 2014:

Hi,

Here’s what your branch looks like:

*   28b9e1e - (HEAD, robertogagliotta/my_sql_upgrade) Merge branch 'my_sql_branch' (11 days ago) <ROBERTO VASQUEZ>
|\  
| * 04d32ca - Goal is to collect this version in sql_upgrade.php script via a sql query and use it as the default setting in the select control (11 days ago) <ROBERTO VASQUEZ>
* |   3c10ba9 - Merge branch 'sql_upgrade' (11 days ago) <ROBERTO VASQUEZ>
|\ \  
| |/  
|/|   
| * 8c19f20 - Goal is to collect this version in sql_upgrade.php script via a sql query and use it as the default setting in the select control (11 days ago) <ROBERTO VASQUEZ>
* | a7e4957 - meeen git is tough !!! (11 days ago) <ROBERTO VASQUEZ>
* | 672e23b - Upgrading sql_upgrade.php (11 days ago) <ROBERTO VASQUEZ>
|/  
* 95e1947 - Update sql_upgrade.php (11 days ago) <Roberto Vasquez>
* f2647a8 - Update sql_upgrade.php (11 days ago) <Roberto Vasquez>
* fb4e4db - (tony/master, oemr/master) Date functions fixes for case statement to match Spanish, Spanish (Latin American), or Spanish (Spain). (2 weeks ago) <Ian Wallace>

Above is done with following command:

git log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit --date=relative

Do the following to flatten it into one commit (will cherry-pick all your non-merge commits):

# (check out your branch)
git checkout my_sql_upgrade
# (copy it to a new branch and go there)
git checkout -b improve-upgrade-script_1
# First, reset branch to commit prior to merge commits
git reset --hard 95e1947
# Now bring in the non-merge commits
git cherry-pick 672e23b
git cherry-pick a7e4957
git cherry-pick 04d32ca
# Now try the above git log... command above and you'll see how the branch is more clear
# Now, do the interactive rebase (the number to right of HEAD is how many commits you plan to squash together)
git rebase -i HEAD~5
# (in the edit screen keep "pick" on the top item, and change the others from "pick" to "squash")
# (on next screen change to one comment "Automating sql_upgrade.php script, take 2)
# You now have a branch with all your commits in one, which can push to github

-brady
OpenEMR

robertovasquez wrote on Sunday, June 15, 2014:

improve-upgrade-script_1 branch has being push to Github

bradymiller wrote on Monday, June 16, 2014:

Hi Roberto,

Almost there :slight_smile:

You still need to do the “git rebase -i HEAD~5” command (see above post) to do an interactive rebase to condense the 5 commits into one.

Here’s what it should look like(note it is all in one commit):

Recommend trying again. These git skills, especially the interactive rebase, are super useful to know.

-brady
OpenEMR

robertovasquez wrote on Tuesday, June 17, 2014:

Thanks Brady
Git is the right and just tool. It is not easy for me to follow Mr. Torvalds. There is a relation between the numbers that I see in the http://git-scm.com/
git log graph and the numbers git cherry-pick

I will

robertovasquez wrote on Saturday, June 21, 2014:

Hi Brady.
the commits has being condese in one, please check the link out and let me know if it is done.

bradymiller wrote on Sunday, June 22, 2014:

Hi Roberto,

There are 3 commits there. Goal is to have only one.

Try and try again :slight_smile:

Guessing the problem is with the interactive rebase (see above last step). Try again and if still having issues, then let us know exact steps you are taking.

By the way, the numbers are sha1 hashes, which is how git tracks everything. Every commit (actually every action also) can be identified by the sha1 hash.

-brady

robertovasquez wrote on Sunday, June 22, 2014:

Thanks Brady
The sha1 hashes numbers change after rebase the only one that do not change is
95e1947 - Update sql_upgrade.php; so I use your logic above but with the new commits sha1 hashes numbers.
Know, I the 2 top numbers are bef021d and 95e1947 my plan is to do:
git reset --hard 95e1947
git cherry-pick bef021
git rebase -i HEAD~2
after USA vs Portugal game

robertovasquez wrote on Monday, June 23, 2014:

Hi Brady
I was wrong, now I know that nothing is changed or destroy in git (Mr. Torvalds is good) ; the sha1 hashes you state above are in git log. (I fail to find them). I follow your commands above.
Please take a look of this link


Thanks
p.s USA 2 Portugal 2

bradymiller wrote on Sunday, June 29, 2014:

Hi Roberto,

Looks to be a good starting point. I placed a review on github:

-brady
OpenEMR