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

Feature enhancements to the resource routing tab #60

Open
wants to merge 176 commits into
base: master
Choose a base branch
from

Conversation

jcraitz
Copy link

@jcraitz jcraitz commented Oct 13, 2015

Hi all,
This is my first pull request, so please pardon my errors and feel free to critique. I work at NCSU Libraries and we added some features to the routing tab for individual resources. The short version is that you now will have the ability to delete steps and also to reassign steps to different groups. Note: these changes will only affect the current resource. The changes will not be saved as a workflow template and restarting the workflow, will reset it to the original template.

It's the end of the day, so I'm just going to throw this up here now for you to check out, and I can add notes tomorrow on some of the details involved in these new features.

I did most of this work a couple of months back, and only recently merged them in to the current version of CORAL. As a result, there may be a few things that I do in a deprecated manner. In particular I haven't had a chance to double check the way I did the database query or the resourcestepForm.js. If you have a moment, will someone please look over these. I'll try to find time tomorrow to go back over them.

Cheers,
Jason Raitz

Cesarfr added 30 commits April 3, 2015 14:49
…rding to the language used and internationalization functions moved to a separate file
…rces

Updated this respository to add the new features
@benheet
Copy link

benheet commented Oct 15, 2015

@jcraitz Would you also have Yan sign up for a github account so that she can participate in the comment thread on this pull request too? Thank you.

@jcraitz
Copy link
Author

jcraitz commented Oct 16, 2015

Sorry for the delay.

Here are some screen shots to show how this ends up changing the routing tab.

2 new columns (pictured below):

  • Between the Title and group columns you can now see a small edit step link. I used an icon that was already in the project. Clicking this link brings up a modal to choose a new group with the option to assign all later steps to that same group.
  • There is now a Delete column at the end of the row. Before actually deleting this step, a confirmation dialog will appear.

Note: I know that the Committee would like to keep this table uncluttered. I feel that these two small icon-sized link columns are a minimal addition that give much needed resource-level flexibility to the routing tab.
newroutingtable

The screenshots below show the modal dialog that appears when a user clicks on the edit step link.

editresourcestepmodal
editstepmodalgroupselection

This is the confirmation dialog that appears when a user clicks a delete step link.
deletestepalert

Some points on logic and clarifications:

  • The big picture here is that we would like to use the currently available new resource workflows as templates. Once we've started a resource workflow, our new features allow users to modify the workflow by deleting steps or editing Groups for that specific resource. This allows the resource to be flexible since they're all special snowflakes. :-)
  • Currently, these workflow changes are not saved or archived anywhere other than the current resource. Consequently, if a user restarts the workflow or deletes a step, there is no way to bring back any changes made since the resource was created from the template.
  • If a step is deleted that later step(s) depended on, the later steps are started and their groups are notified.
  • If the deleted step is the only remaining active step, deleting it will complete the workflow.
  • Speaking briefly to the feedback we've received on our other project, we feel that this ability to alter resource workflows in place, independent of similar workflows, is really valuable and can coexist with the proposed option to choose a new template when restarting a current workflow. We really like that idea BTW.

Ideas for future work:

  • Custom message for reassignments. As the code is written, when a step is reassigned, there is a new reassignment email sent to the new step group. Users have pointed out that this can be confusing for the ones getting the reassignment, as they're not always given any context. To avoid a separate email from the previous group, we propose that a temporary message input is added to the modal to be included in the reassignment email. There is no reason for this note to be saved anywhere, it is merely to give the new group some context on why they now have a new step to complete.
  • Adding new steps. Since we can now delete steps, it would be reasonable for later work to add in the complimentary ability to create new steps on the fly.
  • Archiving altered workflows. This pull request only changes the routing workflow for the current resource. As such, these changes are only visible for the current process. If the resource is deleted or restarted, then these changes disappear. Looking to the proposed feature that would give users new template options when restarting a workflow, it seems reasonable to be able to save a current workflow as a new template. Separately, it might also be valuable to users to archive a modified workflow for record keeping.

@xsong9
Copy link

xsong9 commented Jan 15, 2016

Can Steering Committee review the pull request and let us know if you have any question? Thanks!
--Yan

@xsong9
Copy link

xsong9 commented Jan 21, 2016

Ben, yes, we've been using the enhancement in production since last summer. Beside monograph unit, our serials unit have been using it for our new order workflows. We are very pleased with the enhancements. Let me know if you have more question.

@PaulPoulain
Copy link
Contributor

This PR has conflicts, so I can't move it to the new merged repo. Please resubmit a mergeable PR, and I'll move it to https://github.com/Coral-erm/Coral

@xsong9
Copy link

xsong9 commented Mar 10, 2016

We'll work on it asap. What's your timeline for merging our pull request?

Thanks!
Yan

On Thu, Mar 10, 2016 at 7:39 AM, Paul Poulain notifications@github.com
wrote:

This PR has conflicts, so I can't move it to the new merged repo. Please
resubmit a mergeable PR, and I'll move it to
https://github.com/Coral-erm/Coral


Reply to this email directly or view it on GitHub
#60 (comment).

Xiaoyan Song

E-Resource Librarian
North Carolina State University Libraries
xsong9@ncsu.edu
(919)515-5742

@xsong9
Copy link

xsong9 commented Mar 10, 2016

Hi Paul,

Jason Raiz from our IT department is working on the update now. I'm cc'ing
him here in cause he has any question.

Thanks,
Yan

On Thu, Mar 10, 2016 at 7:39 AM, Paul Poulain notifications@github.com
wrote:

This PR has conflicts, so I can't move it to the new merged repo. Please
resubmit a mergeable PR, and I'll move it to
https://github.com/Coral-erm/Coral


Reply to this email directly or view it on GitHub
#60 (comment).

Xiaoyan Song

E-Resource Librarian
North Carolina State University Libraries
xsong9@ncsu.edu
(919)515-5742

# Conflicts:
#	ajax_htmldata/getRoutingDetails.php
@jcraitz
Copy link
Author

jcraitz commented Mar 11, 2016

I'm still testing this last commit. Feel free to point out anything wrong that you see.

@PaulPoulain
Copy link
Contributor

Le 10/03/2016 15:03, xsong9 a écrit :

We'll work on it asap. What's your timeline for merging our pull request?
I have no timeline, I'll merge it when it's done.
The only problem if it's too long, being that patches could be merged on
the merged-repo, and this #60 wouldn't apply anymore.
So the sooner the better (less risk) !

Thanks!
Yan

Paul Poulain, Associé-gérant / co-owner
BibLibre, Services en logiciels libres pour les bibliothèques
BibLibre, Open Source software and services for libraries

@xsong9
Copy link

xsong9 commented Apr 4, 2016

I don't see #60 in the merged pull request list here:
https://github.com/Coral-erm/Coral/issues

It looks like all conflicts have been resolved. Has this been merged yet? If not, what's the timeline for merging this? Thanks!

@PaulPoulain
Copy link
Contributor

PR ported to the unique repository & merged here : coral-erm/coral#13

Note that there was a missing _() on a javascript string that was needed for translation

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.

9 participants