GSoC-2020: Switching from XML Mappings to Annotations on OpenMRS domain Objects

I’ve just added a comment on your PR @alinmihaila99.

Thanks @ruhanga and @herbert24 !

1 Like

Welcome @alinmihaila99, nice to meet you!

I haven’t been involved in this to date, so sorry if this has already been mentioned, but I saw this thread and it just reminded me of some issues we had which migrating to annotations recently. There were some changes we ended up having to revert. I forget the details, but it would be good to review these tickets (which were all reverted). Thanks!

I believe there are some helpful Talk posts linked in those threads as well.

Take care! Mark

fyi @wyclif @mksd

1 Like

Thanks for support @mogoodrich!

Hi @ruhanga @ibacher @mogoodrich. I have a misunderstanding with the class FieldAnswer and its mapping, where is used a composite Id. I found a few examples, but from here I undrstand I have to create other class for id. Is this the right way to do that?

@alinmihaila99 you could try something along the following lines;

@Entity
public class FieldAnswer extends BaseOpenmrsObject {
    @EmbeddedId
    private Pk id;
    .....
    .....
    .....
    @Embeddable
    private static class Pk {
        @ManyToOne
        @JoinColumn(name="concept_id")
        @Column(name="answer_id")
        private Concept concept;
 
        @ManyToOne
        @JoinColumn(name="field_id")
        private Field field;
        ...
    }
}

Don’t forget to add the appropriate constructors, setters and getters.

1 Like

Thanks a lot, @ruhanga!

Hi @ruhanga, sorry, but I forgot to update the sheet with the classes I annotated. I did it now and it is up to date

1 Like

Thanks for the good work you are doing @alinmihaila99. :slightly_smiling_face:

2 Likes

I have one missunderstanding with the class Provider. This class extends BaseCustomizableMetadata, and in this class there is one filed called attributes. In Provider’s mapping there is the mapping for this field, but it didn’t exist in the class. I added it there like I saw in other annotated class(not by me), the class Visit. Is this te right way to do that? Or is it other way? @ruhanga @ibacher

@alinmihaila99 I think the current version of Visit is wrong. IMO the mapping for attribute should look like this:

@Override
@Access(AccessType.PROPERTY)
@OneToMany(mappedBy = "visit", cascade = CascadeType.ALL, orphanRemoval = true)
@OrderBy("voided asc")
@BatchSize(size = 100)
public Set<VisitAttribute> getAttributes() {
	return super.getAttributes();
}

@Override
public void setAttributes(Set<VisitAttribute> attributes) {
	super.setAttributes(attributes);
}

Or, even better, we should figure out how to add the mapping to the BaseCustomizableMetadata class itself.

2 Likes

Thanks a lot @ibacher. I did this a few minutes ago and it works:). Should I also do the same changes for the class Visit?

Hi @ruhanga, I saw there are not all issues added to the epic. Should I add them or is it enough to be linked to the epic?

Thanks @alinmihaila99, I’d rather link the relevant jira issues to the epic. How would you like to add the issues to the epic though?

You can create a ticket detailing this change. I’d say yes, go ahead. :slight_smile:

Hi @ruhanga @ibacher, I have a problem with class Form Resource. After annotating it, looks like this. The problem is that one test from class UserServiceTest fails after running mvn clean install, but if I run it from IntelIJ it pass. The test is here. I looked in the specified XML file and I think that it don’t load as it is. Do you have any ideas what’s the problem with this test? I let the error log after running mvn clean install here

I just managed to resolve this error and created a PR for this class.

Good that you’ve actually managed to resolve your blocker @alinmihaila99. You can as well report how you resolved it for reference by others that might come up with the same issue.

1 Like

I tried so many things and I really don’t know what solved the issue. Sorry

I need some help with one field from class OrderType. I really don’t know how its mapping should look with annotations. I let you here the hbm mapping: https://pastebin.com/z74bxNsL. I didn’t find some useful things on google. @ruhanga @ibacher. Thanks for support!:slight_smile:

Hi @ruhanga, do you have any ideas if there is one Pull Request for a class with a mapping that includes other subclasses? I am working at class Order, and it has 2 joined sublcasses.