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

remove secret from folder #3

Merged
merged 3 commits into from Dec 18, 2016
Merged

Conversation

agix
Copy link
Contributor

@agix agix commented Dec 16, 2016

Replace moveSecretToFolder by addSecretToFolder because move means deletion from the previous folder and would be a different action (done later).

New actions removeFromCurrentFolder was written to put back one secret/folder in ROOT directory and unshare it with everybody accessing the folder.

@agix agix requested a review from Calyhre December 16, 2016 16:31
'createSecretUserRightsSuccess',
'createSecretUserRightsFailure',
'updateSecretUserRightsSuccess',
'updateSecretUserRightsFailure',
'deleteSecretUserRightsSuccess',
'deleteSecretUserRightsFailure',
'removeSecretFromCurrentFolderSuccess',
'removeSecretFromCurrentFolderFailure',
'moveSecretToFolderSuccess',
'moveSecretToFolderFailure',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please cleanup unused/commented code. Even if we plan to re-use it later, git is here to retrieve it if we need :)

@@ -43,6 +48,9 @@ class MetadataActions {
return secretin
.deleteSecret(secret.id)
.catch((error) => {
this.deleteSecretFailure({
currentUser: secretin.currentUser,
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I now know we can easily have metadata, please return only those. Current user is useless here :)
ℹ️ Also for the success action.

// this.moveSecretToFolderFailure({ error });
// throw error;
// });
// }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this :)

secret,
currentFolderId: parentFolderId,
})}
disabled={!secret.canBeUpdatedBy(currentUser) || typeof parentFolderId === 'undefined'}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parentFolderId may be null, undefined, or a string. Probably useless to check the type here?

@@ -38,6 +38,12 @@ class MetadataStore {
);
}

onDeleteSecretFailure({ currentUser }) {
this.setState(this.state
.set('metadata', Immutable.fromJS(currentUser.metadatas).map(secret => Secret.createFromRaw(secret)))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you'll change the return value, you won't need currentUser here ;)

@agix agix merged commit 27550a7 into feature/first-release Dec 18, 2016
@agix agix deleted the remove_from_folder branch December 18, 2016 10:41
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.

None yet

2 participants