TRUNK-4135 Modify DrugOrder.frequency and TRUNK-4052 Add a way to test liquibase changesets #511
Conversation
| insertStatement.setString(10, discontinuedOrder.discontinuedReasonNonCoded); | ||
| insertStatement.setString(11, UUID.randomUUID().toString()); | ||
| insertStatement.setString(12, "DISCONTINUE"); |
There was a problem hiding this comment.
Probably I should store and get it from discontinueOrder object, right?
There was a problem hiding this comment.
I mean this is a required field that cannot be null. I assumed it is always DISCONTINUE here, but I may as well copy the value from the existing discontinuedOrder.
There was a problem hiding this comment.
This changeset is creating DC orders meaning the action is DISCONTINUE as you are doing
|
You missed something, you need to set the order_frequency for existing drug orders |
| @@ -6805,5 +6806,50 @@ | |||
| <comment>Removing discontinued_by from orders</comment> | |||
| <dropColumn tableName="orders" columnName="discontinued_by" /> | |||
| </changeSet> | |||
|
|
|||
| <changeSet id="201401031432-TRUNK-4135" author="rkorytkowski"> | |||
| <preConditions onFail="MARK_RAN"> | |||
|
Never mind, i have seen the code that sets the drug_order.frequency |
|
|
||
| @Test | ||
| public void shouldUpgradeFromClean1_9To1_10() throws IOException, SQLException { | ||
| upgradeTestUtil.upgrade(); |
|
I seem to like the way you modeled the upgrade tests against a database instance |
| <column name="care_setting" type="varchar(50)"> | ||
| <constraints nullable="false"/> | ||
| </column> | ||
| <column name="care_setting" type="varchar(50)" /> |
There was a problem hiding this comment.
@wluyima you said to remove defaultValue and that you fixed the problem by adding not nullable constraint in some later changeset, but I see that you haven't removed nullable="false" here...
There was a problem hiding this comment.
See the changeset with id 201312271826-TRUNK-4156 that adds orders.care_setting?
|
I think it's ready for merging. I have fixed some new 1.10.x changesets, but I didn't add new checksums for them so devs will need to cleanup their dbs. |
| <not> | ||
| <sqlCheck expectedResult="0"> | ||
| SELECT * FROM orders WHERE care_setting IS NULL | ||
| SELECT count(*) FROM orders WHERE care_setting IS NULL |
There was a problem hiding this comment.
I needed to fix this changeset as well.
There was a problem hiding this comment.
Pull the latest changes, that was fixed in github this morning
| @@ -45,19 +46,22 @@ private void createDiscontinueOrders(JdbcConnection connection, List<Discontinue | |||
| connection.setAutoCommit(false); | |||
| insertStatement = connection | |||
| .prepareStatement("Insert into orders(previous_order_id, concept_id, patient_id, encounter_id, " | |||
There was a problem hiding this comment.
It doesn't assign order_number, which also seems like a bug...
There was a problem hiding this comment.
Good catch! I wonder why it doesn't fail
There was a problem hiding this comment.
It doesn't fail, because order_number is nullable right now... We're missing a changeset, which makes it not nullable.
|
Merged the code at c5533f3 |
Once I added a way to test liquibase upgrade I discovered many of new changesets had bugs, which I fixed as well. Please review, but do not merge yet, as I need to add some assertions in my test to be 100% sure everything worked as expected.