{"action":"created","issue":{"url":"https://api.github.com/repos/jembi/bsis/issues/376","labels_url":"https://api.github.com/repos/jembi/bsis/issues/376/labels{/name}","comments_url":"https://api.github.com/repos/jembi/bsis/issues/376/comments","events_url":"https://api.github.com/repos/jembi/bsis/issues/376/events","html_url":"https://github.com/jembi/bsis/pull/376","id":111143996,"number":376,"title":"Add test batch release stage","user":{"login":"bausmeier","id":116090,"avatar_url":"https://avatars.githubusercontent.com/u/116090?v=3","gravatar_id":"","url":"https://api.github.com/users/bausmeier","html_url":"https://github.com/bausmeier","followers_url":"https://api.github.com/users/bausmeier/followers","following_url":"https://api.github.com/users/bausmeier/following{/other_user}","gists_url":"https://api.github.com/users/bausmeier/gists{/gist_id}","starred_url":"https://api.github.com/users/bausmeier/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bausmeier/subscriptions","organizations_url":"https://api.github.com/users/bausmeier/orgs","repos_url":"https://api.github.com/users/bausmeier/repos","events_url":"https://api.github.com/users/bausmeier/events{/privacy}","received_events_url":"https://api.github.com/users/bausmeier/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-10-13T09:39:39Z","updated_at":"2015-10-13T11:30:51Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/jembi/bsis/pulls/376","html_url":"https://github.com/jembi/bsis/pull/376","diff_url":"https://github.com/jembi/bsis/pull/376.diff","patch_url":"https://github.com/jembi/bsis/pull/376.patch"},"body":"This is the first part of #373.\r\n\r\nChanges:\r\n* Adds a RELEASED status to test batches.\r\n* Moves the existing logic for creating deferrals, creating post donation counseling, and flagging components as unsafe to the release stage.\r\n* Adds logic for when a test batch can be closed or released.\r\n\r\nTo be done in the next part:\r\n* Only allow test batches to be closed when there are no discrepancies.\r\n* Only update donation statuses when the test batch is released.\r\n* Only handle test samples with no discrepancies when releasing a test batch.\r\n* Automatically release individual test samples when discrepancies are resolved and the test batch is released.\r\n\r\n@danfuterman Please review."},"comment":{"url":"https://api.github.com/repos/jembi/bsis/issues/comments/147689556","html_url":"https://github.com/jembi/bsis/pull/376#issuecomment-147689556","issue_url":"https://api.github.com/repos/jembi/bsis/issues/376","id":147689556,"user":{"login":"danfuterman","id":1052668,"avatar_url":"https://avatars.githubusercontent.com/u/1052668?v=3","gravatar_id":"","url":"https://api.github.com/users/danfuterman","html_url":"https://github.com/danfuterman","followers_url":"https://api.github.com/users/danfuterman/followers","following_url":"https://api.github.com/users/danfuterman/following{/other_user}","gists_url":"https://api.github.com/users/danfuterman/gists{/gist_id}","starred_url":"https://api.github.com/users/danfuterman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danfuterman/subscriptions","organizations_url":"https://api.github.com/users/danfuterman/orgs","repos_url":"https://api.github.com/users/danfuterman/repos","events_url":"https://api.github.com/users/danfuterman/events{/privacy}","received_events_url":"https://api.github.com/users/danfuterman/received_events","type":"User","site_admin":false},"created_at":"2015-10-13T11:30:51Z","updated_at":"2015-10-13T11:30:51Z","body":"Thanks @bausmeier, this looks like you're on the right track. A few points I'd like to raise for discussion after looking at the PR:\r\n\r\n - Looking at the use of `isTestingSupervisor`:\r\n```\r\nboolean isTestingSupervisor = PermissionUtils.loggedOnUserHasPermission(PermissionConstants.EDIT_TEST_BATCH);\r\n```\r\nI wonder if I got this wrong in the initial spec. The aim of this is to limit certain actions/operations to users with the supervisor role, but my thought is maybe this would be better handled by some new specific, custom permissions, rather than at the role level. The problem is that Roles can be configured by users - having the `EDIT_TEST_BATCH` permission doesn't guarantee that the user is in fact a testing supervisor. What do you think of adding in new permissions that are more specific to the operations being added here, rather than just using `EDIT_TEST_BATCH` e.g. `RELEASE_TEST_BATCH`, `CLOSE_TEST_BATCH`.\r\n\r\n - Looking at TestBatchStatus (https://github.com/jembi/bsis/pull/376/files#diff-1cc564d82b9f6bde5bd9cfb65567d3ebL4), there are a few batch states that aren't represented here that could be useful to the end-user - it might be better to dynamically infer these, rather than have them as explicit batch statuses, but I'd like to raise this for discussion. I have this UI in mind when thinking about this, where it would be useful to see info about the state of open test batches:\r\n![image](https://cloud.githubusercontent.com/assets/1052668/10453540/5909db16-71ad-11e5-9e54-e7ed16e65df4.png)\r\n\r\n - Before being released, it would be useful if there was a way to show if a batch is ready to be released or not\r\n - After being released, it would be useful if there was a way to show if a batch is ready to be closed or not\r\n\r\nCurrently, I manage the last one in the frontend by checking on the state of testing (https://github.com/jembi/bsis-frontend/blob/master/app/views/testing/viewTestBatch.html#L12) - \r\n```\r\n <span ng-if=\"!pendingBloodTypingTests && !pendingTTITests \r\n    && basicBloodTypingComplete && basicTTIComplete\r\n    && !pendingBloodTypingMatchTests\">\r\n```\r\n\r\nBut it might be better to manage/determine this in the backend, and pass that info through to the frontend.\r\n\r\nFinally, I need to give it some more thought, but it might be useful to also represent the states prior to release where the batch is reviewed and validated."}}