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

PTM-82: Functionality to load patients considering the date created and date changed #34

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@Lahiru-J
Member

Lahiru-J commented Jun 21, 2017

For the incremental patient matching, records of the patients are
loaded considering the date of a particular report created against with
the patient’s date_created or date_changed

@dkayiwa

This comment has been minimized.

Member

dkayiwa commented Jun 22, 2017

@Lahiru-J do you mind squashing the commits into one as advised at? https://wiki.openmrs.org/display/docs/Pull+Request+Tips

@Lahiru-J

This comment has been minimized.

Member

Lahiru-J commented Jun 22, 2017

Done @dkayiwa

import java.util.Collection;
import java.util.Date;
public class OpenMRSReaderTest extends BaseContextSensitiveTest {

This comment has been minimized.

@dkayiwa

dkayiwa Jun 22, 2017

Member

The base class should be BaseModuleContextSensitiveTest

This comment has been minimized.

@Lahiru-J

Lahiru-J Jun 23, 2017

Member

Okay I will change this. Thank you for pointing me that

<!-- Base Module Properties -->
<id>@MODULE_ID@</id>
<name>@MODULE_NAME@</name>

This comment has been minimized.

@dkayiwa

dkayiwa Jun 22, 2017

Member

Did you add this file accidentally?

This comment has been minimized.

@Lahiru-J

Lahiru-J Jun 23, 2017

Member

config.xml file was needed to be added as I used BaseContextSensitiveTest instead of BaseModuleContextSensitiveTest. This one also has been solved by now

/**
* Constant name is given for a report in the incremental patient matching process
*/
private static final String REPORT_NAME = "dedup-incremental-report-";

This comment has been minimized.

@bmamlin

bmamlin Jul 4, 2017

Member

Is this a report name or a report name prefix?

This comment has been minimized.

@Lahiru-J

Lahiru-J Jul 14, 2017

Member

Yes it is a report name prefix. I'll change the variable name

updatePatientList();
log.info("Finish intialization ...");

This comment has been minimized.

@bmamlin

bmamlin Jul 4, 2017

Member

Assuming this is noting completion of initialization, perhaps "Finished initialization."

@@ -0,0 +1,21 @@
<!DOCTYPE hibernate-mapping PUBLIC "-//Hibernate/Hibernate Mapping DTD 3.0//EN" "http://hibernate.sourceforge.net/hibernate-mapping-3.0.dtd">
<hibernate-mapping>
<class name="org.openmrs.module.patientmatching.ConfigurationEntry" table="patientmatching_configurationEntries">

This comment has been minimized.

@bmamlin

bmamlin Jul 4, 2017

Member

Our convention for table names is lowercase with or without underscores as needed to separate words, so the table name patientmatching_configurationentries or patientmatching_configuration_entries would more closely follow conventions.

This comment has been minimized.

@Lahiru-J

Lahiru-J Jul 14, 2017

Member

@bmamlin this has a slight problem because the default table name is patientmatching_configurationEntries, which is provided by the OpenMRS database. So I guess I cannot change it can I?

This comment has been minimized.

@bmamlin

bmamlin Jul 17, 2017

Member

Ah, so you didn't come up with this table name; rather, you are inheriting it from earlier authors of the module. This table belongs to the patient matching module (our convention is to prepend table names with module ID to avoid conflicts), so it would be possible to rename the table with a liquibase changeset. When I saw the hibernate mapping file, I assumed you were creating the table. If the table already exists for the module, then we can address changing the table name to follow conventions at a later time.

@dkayiwa

This comment has been minimized.

Member

dkayiwa commented Jul 14, 2017

@Lahiru-J did you see the above comments? 😊

@Lahiru-J

This comment has been minimized.

Member

Lahiru-J commented Jul 14, 2017

@dkayiwa sorry it took some time because I was hospitalized for the passed few days due to dengue fever

@dkayiwa

This comment has been minimized.

Member

dkayiwa commented Sep 18, 2017

@Lahiru-J sorry about that man!
Are you now back on your feet? 😊

@dkayiwa

This comment has been minimized.

Member

dkayiwa commented Nov 15, 2017

@Lahiru-J do you still have time to take this forward?

@Lahiru-J

This comment has been minimized.

Member

Lahiru-J commented Nov 16, 2017

yes @dkayiwa,
I have completed all the required changes to this ticket (PTM-82) and I am waiting until this get fully reviewed so I can fix them accordingly.

@dkayiwa

This comment has been minimized.

Member

dkayiwa commented Jun 20, 2018

@Lahiru-J are you still interested in finishing this up?

@Lahiru-J

This comment has been minimized.

Member

Lahiru-J commented Jun 21, 2018

@dkayiwa Yes, as I mentioned in the previous comment I am still waiting until this get reviewed

@dkayiwa

This comment has been minimized.

Member

dkayiwa commented Jul 12, 2018

@Lahiru-J can you always include a link to the ticket as advised at? https://wiki.openmrs.org/display/docs/Pull+Request+Tips

@dkayiwa

This comment has been minimized.

Member

dkayiwa commented Jul 16, 2018

Ping me to reopen this when you are ready to continue working on it.

@dkayiwa dkayiwa closed this Jul 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment