-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[#TRUNK-4202] Convert Order.orderer to be a Provider #563
Conversation
ON o.orderer = u.user_id INNER JOIN provider p ON p.person_id = u.person_id) AS b ON o.orderer = b.orderer | ||
SET o.orderer = b.provider_id | ||
</sql> | ||
</changeSet> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a unit test to Database1_9To1_10UpgradeTest to ensure that this works as expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wluyima : i have checked whether i could see any documentation inform of parhaps wiki pages about; "unit testing for our changesets" and i didn't get one, it looks to me like this is still under development as can be seen @{https://tickets.openmrs.org/browse/TRUNK-4052}, in case am not right in this manner, then i request to be directed to that way in which i can have my changeset tested according to openmrs standards
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See Database1_9To1_10UpgradeTest.java and add your tests to it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
am challenged to write a changeset that; shouldMigrateDataInOrdererColumnToBeProviderIdsInsteadOfUserIds, since am not sure of which way to use in order to run my changeset inside the testcase to update the datasets available within openmrs now that this is under development
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look at the tests in that class, yours will be very similar
|
||
<changeSet id="201401161532-TRUNK-4202" author="k-joseph" dbms="mysql"> | ||
<preConditions onFail="MARK_RAN"> | ||
<columnExists tableName="orders" columnName="orderer" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This columnExists is not necessary
here is my query; the other query looks also like; i think i got some mismatches within the local as compared to the origin branch, which i tried to resolve by pulling first from upstream and then also from origin, which after doing i tried to push i had to resolve by resolving some conflicts, this i did successfully and unfortunately, this resulted into some failure when trying to push some changes to the origin branch, here is the failure trace; so as am still clearing the issues in the queries i kindly ask better review of them by devs as @wluyima here so that i can fully clear up the failures in the test, otherwise am trying to resolve this push rejected failure issue, regards |
The query that updates orderer i think can written in a cleaner and easier way to understand by just joining directly to the provider and user tables without that subselect which makes things a little harder to read. About the push, you must have rebased something you had already pushed to the 1.10.x branch in your fork and that is why git is rejecting the push to your fork, you can add -f when pushing to force the push to your fork |
And in the INSERT query, when selecting the user_id for daemon, can you use the uuid in the WHERE clause instead of the system_id(deamon), it is more reliable to use the uuid |
i have resolved the uuid issue. and the query is; INSERT INTO provider(person_id, identifier, creator, date_created, retired, uuid) and then the other query is; UPDATE orders AS o INNER JOIN provider p ON o.orderer = p.provider_id however the test is still failing with the same message as before, any other thing i need to adjust should be pointed out here so that i can clear it as well and have the test pass, otherwise as @wluyima suggests am looking forward to make sure that the test passes however i agree that my sql knowledge is just being improved and that is partly the reason why am even taking alot of time on this issus than was originally planned and expected, sorry for any inconveniences this delay has caused, |
No problem, we are all always learning new things and keep improving. |
We can pair later today if you still have issues |
Merged at 0ea76fa |
https://tickets.openmrs.org/browse/TRUNK-4202 | created provider accounts for orderer s, migrated orderer s to be provider ids instead of user ids