Fix #905. Implemented user manager plugin #1061
Conversation
* Created also a generic confirm dialog * Reload page on user create/delete
* Groups Management * Various fixes
| @@ -0,0 +1,615 @@ | |||
| /** | |||
There was a problem hiding this comment.
Why do you have importer related actions in this PR?
| return (dispatch, getState) => { | ||
| let text = searchText; | ||
| let state = getState && getState(); | ||
| if (state) { |
There was a problem hiding this comment.
I don't like too much handling this sort of stuff here
There was a problem hiding this comment.
Yes I know that search text and current page should come from the action, but this way the component are not indipendent. Where you should place them?
| groups | ||
| }); | ||
| }); | ||
| dispatch({ |
There was a problem hiding this comment.
define an action creator, please
There was a problem hiding this comment.
Didn't find any convention that forces to create module internal function that create the final object from the action creator. I thought to improve readability in this way (avoid to jump here and there for simple objects definition, avoid passing variables that can be inverted and so on...) I fixed this, let's check if it is a good convention and lets write it if there is a reason to make it mandatory.
There was a problem hiding this comment.
The only reason for now is: we always did this way, so far, let's discuss if we want to improve it and how, but I agree it's not mandatory
|
|
||
| }); | ||
| }).catch((error) => { | ||
| dispatch({ |
There was a problem hiding this comment.
define an action creator, please
| status: "loading" | ||
| }); | ||
| return API.getAvailableGroups(user).then((groups) => { | ||
| dispatch({ |
There was a problem hiding this comment.
define an action creator, please
| const {editUser, changeUserMetadata, saveUser} = require('../../../actions/users'); | ||
|
|
||
|
|
||
| const mapStateToProps = (state) => { |
There was a problem hiding this comment.
Do we need to make it so complex? Please, use the usual selectors composition with reselect
There was a problem hiding this comment.
They are only not null checks, how it is less complex and more readable than this?
| groups: users && users.groups | ||
| }; | ||
| }; | ||
| const mapDispatchToProps = (dispatch) => { |
There was a problem hiding this comment.
bindActionCreators is not needed, bind is not needed
There was a problem hiding this comment.
line 25, bind the action with null as first parameter, this is needed.
There was a problem hiding this comment.
ok, got it, do we also need it for onChange and onSave ?
| onDelete: deleteUser | ||
| }, dispatch); | ||
| }; | ||
| const mergeProps = (stateProps, dispatchProps, ownProps) => { |
There was a problem hiding this comment.
Not clear the purpose of this...
There was a problem hiding this comment.
allow overriding from ownProps , start and limit , even if not defined.
| const assign = require('object-assign'); | ||
| function users(state = { | ||
| start: 0, | ||
| limit: 12 |
There was a problem hiding this comment.
12 is duplicated everywhere, can we make this a constant that we change in a single place?
| top: "50%", | ||
| left: "50%", | ||
| transform: "translate(-50%, -40%)" | ||
| }}>Loading...<Spinner spinnerName="circle" noFadeIn/></div></div>); |
Implemented user manager.