Skip to content
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

RESTWS-641: Add support for MIME types when posting complexObs value in json request #360

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ruhanga
Copy link
Member

@Ruhanga Ruhanga commented Nov 16, 2018

Hello reviewer,

I'd like to have this reviewed.

link to ticket https://issues.openmrs.org/browse/RESTWS-641

@Ruhanga Ruhanga changed the title Add support for MIME types when posting complexvalue in json request Add support for MIME types when posting complexObs value in json request Nov 16, 2018
@Ruhanga Ruhanga changed the title Add support for MIME types when posting complexObs value in json request RESTWS-641: Add support for MIME types when posting complexObs value in json request Nov 16, 2018
@dkayiwa
Copy link
Member

dkayiwa commented Nov 17, 2018

Can you include the ticket id in the commit message as required at? https://wiki.openmrs.org/display/docs/Pull+Request+Tips

@Ruhanga
Copy link
Member Author

Ruhanga commented Nov 18, 2018

My screen shows me the ticket Id is included. (As in, "RESTWS-641: Add support for MIME types when posting complexObs value in json request" ). I also tried to change the message with git commit --amend. Hope the ticket id appears now. Or may maybe am missing some obvious point, any help is highly appreciated. Thank you.

cc @dkayiwa

@dkayiwa
Copy link
Member

dkayiwa commented Nov 21, 2018

Can you look at the commit message from here? d1c9557

import java.io.InputStream;
import java.util.List;
import java.util.Locale;
import static org.junit.Assert.*;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conventions followed @dkayiwa.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved.

@dkayiwa
Copy link
Member

dkayiwa commented Dec 3, 2018

@Ruhanga do all tests in this commit pass with your changes? f8a61f0

@Ruhanga
Copy link
Member Author

Ruhanga commented Dec 5, 2018

@dkayiwa , yes Sir.

@Ruhanga Ruhanga force-pushed the RESTWS-641 branch 2 times, most recently from b7331f9 to 696bb81 Compare December 20, 2018 19:35
ComplexData complexData = new ComplexData(obs.getUuid() + ".raw", new ByteArrayInputStream(bytes));
String extension = "raw";
//Try to extract extension from the MIME type if it is provided in value
if (value.toString().indexOf("data:") == 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above test is a bit confusing at the first glance. I thought you are testing for absence of mime type with a return value of 0. It took me time to discover that you are actually checking if it starts with it. Don't you think startsWith would be clearer?

@ibacher ibacher force-pushed the master branch 2 times, most recently from f9c73bb to 2a4407f Compare December 14, 2020 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants