-
Notifications
You must be signed in to change notification settings - Fork 35
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
Review workflow UI improvements. #319
Changes from 8 commits
a8176d3
7168d51
99b9a47
7752bc2
b8c2e61
790f72c
f32c6ad
b4aadb3
01240de
34f9f23
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,44 @@ import AdminLink from "../../components/AdminLink"; | |
import { ProgressBar, ProgressStep } from "./ProgressBar.js"; | ||
|
||
|
||
function isMember(groupkey, source, sessionState, bucketState) { | ||
const {serverInfo: {user={}, capabilities}} = sessionState; | ||
if (!user.id) { | ||
return false; | ||
} | ||
const {signer={}} = capabilities; | ||
const {[groupkey]: defaultGroupName} = signer; | ||
const {[groupkey]: groupName=defaultGroupName} = source; | ||
const {id: userId} = user; | ||
console.log(bucketState); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. leftover |
||
const {groups} = bucketState; | ||
const editorGroup = groups.find(g => g.id === groupName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
if (editorGroup == null) { | ||
// XXX for now if we can't access the group it's probably because the user | ||
// doesn't have the permission to read it, so we mark the user has a member | ||
// of the group. | ||
// Later when https://github.com/Kinto/kinto/pull/891/files lands, we'll | ||
// have access to the principals attached to a given authentication, so we'll | ||
// be able to properly check for membership. | ||
return true; | ||
} | ||
return editorGroup.members.includes(userId); | ||
} | ||
|
||
function isEditor(source, sessionState, bucketState) { | ||
return isMember("editors_group", source, sessionState, bucketState); | ||
} | ||
|
||
function isReviewer(source, sessionState, bucketState) { | ||
return isMember("reviewers_group", source, sessionState, bucketState); | ||
} | ||
|
||
function isLastEditor(source, sessionState) { | ||
const {serverInfo: {user={}}} = sessionState; | ||
const {lastEditor} = source; | ||
return user.id === lastEditor; | ||
} | ||
|
||
export default class SignoffToolBar extends React.Component { | ||
props: { | ||
sessionState: SessionState, | ||
|
@@ -59,32 +97,38 @@ export default class SignoffToolBar extends React.Component { | |
} | ||
|
||
const {source, preview, destination} = resource; | ||
const canRequestReview = canEdit && isEditor(source, sessionState, bucketState); | ||
const canReview = canEdit && | ||
isReviewer(source, sessionState, bucketState) && | ||
!isLastEditor(source, sessionState); | ||
const canSign = canEdit && isReviewer(source, sessionState, bucketState); | ||
|
||
// Default status is request review | ||
const step = status == "to-review" ? 1 : status == "signed" ? 2 : 0; | ||
return ( | ||
<ProgressBar> | ||
<WorkInProgress label="Work in progress" | ||
step={0} | ||
currentStep={step} | ||
canEdit={canEdit} | ||
requestReview={requestReview} | ||
source={source} /> | ||
<WorkInProgress | ||
label="Work in progress" | ||
step={0} | ||
currentStep={step} | ||
canEdit={canRequestReview} | ||
requestReview={requestReview} | ||
source={source} /> | ||
<Review label="Waiting review" | ||
step={1} | ||
currentStep={step} | ||
canEdit={canEdit} | ||
approveChanges={approveChanges} | ||
declineChanges={declineChanges} | ||
source={source} | ||
preview={preview} /> | ||
step={1} | ||
currentStep={step} | ||
canEdit={canReview} | ||
approveChanges={approveChanges} | ||
declineChanges={declineChanges} | ||
source={source} | ||
preview={preview} /> | ||
<Signed label="Signed" | ||
step={2} | ||
currentStep={step} | ||
canEdit={canEdit} | ||
reSign={approveChanges} | ||
source={source} | ||
destination={destination} /> | ||
step={2} | ||
currentStep={step} | ||
canEdit={canSign} | ||
reSign={approveChanges} | ||
source={source} | ||
destination={destination} /> | ||
</ProgressBar> | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
import { expect } from "chai"; | ||
import { put } from "redux-saga/effects"; | ||
|
||
import { notifySuccess } from "../../../src/actions/notifications"; | ||
import { routeLoadSuccess } from "../../../src/actions/route"; | ||
import { setClient } from "../../../src/client"; | ||
import * as actions from "../../../src/plugins/signoff/actions"; | ||
import * as saga from "../../../src/plugins/signoff/sagas"; | ||
|
||
|
||
describe("signoff plugin sagas", () => { | ||
describe("handleRequestReview()", () => { | ||
let bucket, collection, getState, handleRequestReview; | ||
|
||
before(() => { | ||
collection = { | ||
getData() {}, | ||
setData() {}, | ||
}; | ||
bucket = {collection() {return collection;}}; | ||
setClient({bucket() {return bucket;}}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: maybe put all that in a |
||
const action = actions.requestReview(); | ||
getState = () => ({ | ||
bucket: {data: {id: "buck"}}, | ||
collection: {data: {id: "coll", last_modified: 42}} | ||
}); | ||
handleRequestReview = saga.handleRequestReview(getState, action); | ||
}); | ||
|
||
it("should update the collection status as 'to-review'", () => { | ||
expect(handleRequestReview.next({id: "coll"}).value) | ||
.to.have.property("CALL") | ||
.to.have.property("args") | ||
.to.include({status: "to-review"}); | ||
}); | ||
|
||
it("should dispatch the routeLoadSuccess action", () => { | ||
expect(handleRequestReview.next({data: {id: "coll", status: "to-review"}}).value) | ||
.eql(put(routeLoadSuccess({ | ||
bucket: {data: {id: "buck"}}, | ||
collection: {data: {id: "coll", status: "to-review"}}, | ||
}))); | ||
}); | ||
|
||
it("should dispatch a success notification", () => { | ||
expect(handleRequestReview.next().value) | ||
.eql(put(notifySuccess("Review requested."))); | ||
}); | ||
}); | ||
|
||
describe("handleDeclineChanges()", () => { | ||
let bucket, collection, getState, handleDeclineChanges; | ||
|
||
before(() => { | ||
collection = { | ||
getData() {}, | ||
setData() {}, | ||
}; | ||
bucket = {collection() {return collection;}}; | ||
setClient({bucket() {return bucket;}}); | ||
const action = actions.requestReview(); | ||
getState = () => ({ | ||
bucket: {data: {id: "buck"}}, | ||
collection: {data: {id: "coll", last_modified: 42}} | ||
}); | ||
handleDeclineChanges = saga.handleDeclineChanges(getState, action); | ||
}); | ||
|
||
it("should update the collection status as 'work-in-progress'", () => { | ||
expect(handleDeclineChanges.next({id: "coll"}).value) | ||
.to.have.property("CALL") | ||
.to.have.property("args") | ||
.to.include({status: "work-in-progress"}); | ||
}); | ||
|
||
it("should dispatch the routeLoadSuccess action", () => { | ||
expect(handleDeclineChanges.next({data: {id: "coll", status: "work-in-progress"}}).value) | ||
.eql(put(routeLoadSuccess({ | ||
bucket: {data: {id: "buck"}}, | ||
collection: {data: {id: "coll", status: "work-in-progress"}}, | ||
}))); | ||
}); | ||
|
||
it("should dispatch a success notification", () => { | ||
expect(handleDeclineChanges.next().value) | ||
.eql(put(notifySuccess("Changes declined."))); | ||
}); | ||
}); | ||
|
||
describe("handleApproveChanges()", () => { | ||
let bucket, collection, getState, handleApproveChanges; | ||
|
||
before(() => { | ||
collection = { | ||
getData() {}, | ||
setData() {}, | ||
}; | ||
bucket = {collection() {return collection;}}; | ||
setClient({bucket() {return bucket;}}); | ||
const action = actions.requestReview(); | ||
getState = () => ({ | ||
bucket: {data: {id: "buck"}}, | ||
collection: {data: {id: "coll", last_modified: 42}} | ||
}); | ||
handleApproveChanges = saga.handleApproveChanges(getState, action); | ||
}); | ||
|
||
it("should update the collection status as 'to-sign'", () => { | ||
expect(handleApproveChanges.next({id: "coll"}).value) | ||
.to.have.property("CALL") | ||
.to.have.property("args") | ||
.to.include({status: "to-sign"}); | ||
}); | ||
|
||
it("should dispatch the routeLoadSuccess action", () => { | ||
expect(handleApproveChanges.next({data: {id: "coll", status: "to-sign"}}).value) | ||
.eql(put(routeLoadSuccess({ | ||
bucket: {data: {id: "buck"}}, | ||
collection: {data: {id: "coll", status: "to-sign"}}, | ||
}))); | ||
}); | ||
|
||
it("should dispatch a success notification", () => { | ||
expect(handleApproveChanges.next().value) | ||
.eql(put(notifySuccess("Signature requested."))); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could even do this in one line without intermediary
capabilities
andsigner
variables :)#PerlFTW
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean
LOLNOPE 😬