opsgenie / opsgenieclient

Apache License 2.0
20 stars 20 forks source link

PARTICIPANT is named "name" #2

Closed r4um closed 11 years ago

r4um commented 11 years ago

Fixes WhoIsOnCall family of functions/interfaces response. In participants array in response, the participant's name has the key 'name' and not 'participant' in the objects.

andrewdotn commented 11 years ago

Hi, I’m running into this same bug. Unfortunately, the participant really is called participant in a ScheduleRule, so changing the value of PARTICIPANT to name would break opsClient.schedule().listSchedules(schedRequest).getSchedules().get(0).getRules().get(0).getParticipants().get(0).getParticipant()) and make it always return null.

Perhaps ScheduleParticipant should be changed to something like this instead:

@Override
public void fromMap(Map map) throws ParseException {
    participant = (String) map.get(OpsGenieClientConstants.API.PARTICIPANT);
    if(participant == null){
        participant = (String) map.get(OpsGenieClientConstants.API.NAME);
    }
    if(map.containsKey(OpsGenieClientConstants.API.TYPE)){
        type = Type.valueOf(((String) map.get(OpsGenieClientConstants.API.TYPE)).toLowerCase());
    }
}

But then I’m noticing other issues with keys not matching between JSON and Java. For example, ScheduleRule is looking for startTime but the JSON contains startDate, so opsClient.schedule().listSchedules(schedRequest).getSchedules().get(0).getRules().get(0).getStartDate() currently returns null.

So maybe there should be a general way to make sure the keys match between JSON and Java, such as some sort of integration test?

enguzekli commented 11 years ago

Actually, we already have several integration tests. I will check why this point is missed.

r4um commented 11 years ago

@andrewdotn good catch! Yes this breaks {List,Get}Schedules. Perhaps the WhoIsOnCallAPI response should be fixed to reply as code intends, to bring uniformity ? (Might break existing stuff for users or internally)

I too confirm the ScheduleRule bug. Just need to replace OpsGenieClientConstants.API.START_TIME with OpsGenieClientConstants.API.START_DATE in ScheduleRule.java for a fix ?

enguzekli commented 11 years ago

Just checked and it seems that integration test for both of these issues are missed. We will release new sdk soon which includes these fixes.

@andrewdotn your fix will be applied in order to make it work with both participant and name.

andrewdotn commented 11 years ago

Awesome, thanks! By the way, how can I get notified when you release a new version of the SDK?

r4um commented 11 years ago

@enguzekli thanks. Closing this PR.

enguzekli commented 11 years ago

@r4um @andrewdotn new release is available. Please give it a try!

andrewdotn commented 11 years ago

Works great, thanks!

r4um commented 11 years ago

@enguzekli Works now, thanks!