Skip to content

Refactor taskAction resource to take in a Task definition uuid in add…#431

Closed
gitcliff wants to merge 1 commit intoopenmrs:masterfrom
gitcliff:RESTWS-764
Closed

Refactor taskAction resource to take in a Task definition uuid in add…#431
gitcliff wants to merge 1 commit intoopenmrs:masterfrom
gitcliff:RESTWS-764

Conversation

@gitcliff
Copy link
Contributor

@gitcliff gitcliff commented May 5, 2020

@gitcliff
Copy link
Contributor Author

@ibacher kindly requesting for your review on this pr

Copy link
Member

@ibacher ibacher left a comment

Choose a reason for hiding this comment

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

@gitcliff It would be helpful here to have some tests to demonstrate (at a minimum) that this meets the requirements of the ticket. Maybe I'm missing something, but I don't see how this PR meets the ask of RESTWS-764.

Comment on lines +74 to +91
case SCHEDULETASK:
scheduleTasks(taskDefinitions);
break;
case RESCHEDULETASK:
reScheduleTasks(taskDefinitions);
break;
case DELETE:
deleteTasks(taskDefinitions);
break;
case SHUTDOWNTASK:
shutDownTasks(taskDefinitions);
break;
case RESCHEDULEALLTASKS:
reScheduleAllTasks();
break;
case RUNTASK:
runTasks(taskDefinitions);
break;
Copy link
Member

Choose a reason for hiding this comment

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

These case statements should be indented one more level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes sure,going to add it

public TaskDefinition getByUniqueId(String uniqueId) {
TaskDefinition taskDefinition = taskServiceWrapper.getTaskByName(uniqueId);
if (taskDefinition == null) {
taskDefinition = taskServiceWrapper.getTaskByUuid(uniqueId);
Copy link
Member

Choose a reason for hiding this comment

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

I can see why we want to add the functionality to get a task by UUID, but do we also want to remove the functionality to get the task by ID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ibacher yes i think its ideal to get a task by its UUID and leave the task by ID for internal database identification and operations

@gitcliff
Copy link
Contributor Author

@ibacher thanks, sure i was going to add the tests .
This pr involves refactoring the Task_Definition resource because the Task_Action resource relies on its getByUniqueId(String uniqueId) of Task_Definition in order to do a POST request of Task_Action with a UUID of a task definition just like the way the getTaskByName(name) of task definition is called into action when doing a POST request of Task_Action with a task definition name..

@gitcliff gitcliff closed this May 15, 2020
@gitcliff gitcliff deleted the RESTWS-764 branch May 15, 2020 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants