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

DS-2520: Modify XPath expression when retrieving a workflow step #976

Merged
merged 1 commit into from Mar 31, 2016

Conversation

FacundoAdorno
Copy link
Contributor

When we have two workflows, W1 and W2, and W2 has a first
step with a ID equal to other step ID in W1, WorkflowFactory
retrieves erroneously the step in W1 when we ask about the
first step in W2.

To prevent this, the corresponding XPath expression was modified.

See JIRA issue for more details.

@tdonohue
Copy link
Member

tdonohue commented Jul 2, 2015

Thanks for the PR, @FacundoAdorno ! It looks reasonable at a glance (though I admit to be less familiar with this area of the code). We just need to find someone to give it a quick test.

@tdonohue tdonohue added the bug label Jul 2, 2015
@tdonohue tdonohue added the merge conflict PR has a merge conflict that needs resolution label Oct 1, 2015
When we have two workflows, W1 and W2, and W2 has a first
step with a ID equal to other step ID in W1, WorkflowFactory
retrieves erroneously the step in W1 when we ask about the
first step in W2.

To prevent this, the corresponding XPath expression was modified.
@FacundoAdorno
Copy link
Contributor Author

Hi! I have solved the merge conflict. Regards!

@KevinVdV KevinVdV added this to the 6.0 milestone Jan 4, 2016
@KevinVdV KevinVdV removed the merge conflict PR has a merge conflict that needs resolution label Jan 4, 2016
@tdonohue tdonohue added the quick win Pull request is small in size & should be easy to review and/or merge label Mar 30, 2016
@hardyoyo
Copy link
Member

+1 by inspection, I have not tested it, but I concur with the analysis on the Jira issue, removing the // from the xpath is required to facilitate the workflow configuration example given on the ticket.

@KevinVdV
Copy link
Member

Took it for a quick test run, works as advertised, so merging.

Thanks @FacundoAdorno

@KevinVdV KevinVdV merged commit f970c04 into DSpace:master Mar 31, 2016
@FacundoAdorno
Copy link
Contributor Author

You are welcome. Regards!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug quick win Pull request is small in size & should be easy to review and/or merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants