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

Allow users to edit and delete their account #2762

Merged
merged 8 commits into from
May 18, 2018
Merged

Conversation

marten
Copy link
Contributor

@marten marten commented May 14, 2018

I think that it might be easiest to keep features like these in Panoptes as HTML rather than APIs. That way we enforce consistency across frontends.

  • Renamed "sign up" to "create account". The suble distinction between "sign up" and "sign in" confuses me every time I look at this screen.

image

image

image

Closes #2410

Review checklist

  • First, the most important one: is this PR small enough that you can actually review it? Feel free to just reject a branch if the changes are hard to review due to the length of the diff.
  • If there are any migrations, will they the previous version of the app work correctly after they've been run (e.g. the don't remove columns still known about by ActiveRecord).
  • If anything changed with regards to the public API, are those changes also documented in the apiary.apib file?
  • Are all the changes covered by tests? Think about any possible edge cases that might be left untested.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

.rubocop.yml: Lint/BlockAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/BlockAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/DefEndAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/EndAlignment has the wrong namespace - should be Layout
.rubocop.yml: Style/ConstantName has the wrong namespace - should be Naming
.rubocop.yml: Style/MethodName has the wrong namespace - should be Naming
.rubocop.yml: Style/PredicateName has the wrong namespace - should be Naming
.rubocop.yml: Style/FileName has the wrong namespace - should be Naming
.rubocop.yml: Style/AccessorMethodName has the wrong namespace - should be Naming
.rubocop.yml: Style/BlockLength has the wrong namespace - should be Metrics
Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in .rubocop.yml
`Style/IfUnlessModifier: MaxLineLength` has been removed. Use `Metrics/LineLength: Max` instead

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

.rubocop.yml: Lint/BlockAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/BlockAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/DefEndAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/EndAlignment has the wrong namespace - should be Layout
.rubocop.yml: Style/ConstantName has the wrong namespace - should be Naming
.rubocop.yml: Style/MethodName has the wrong namespace - should be Naming
.rubocop.yml: Style/PredicateName has the wrong namespace - should be Naming
.rubocop.yml: Style/FileName has the wrong namespace - should be Naming
.rubocop.yml: Style/AccessorMethodName has the wrong namespace - should be Naming
.rubocop.yml: Style/BlockLength has the wrong namespace - should be Metrics
Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in .rubocop.yml
`Style/IfUnlessModifier: MaxLineLength` has been removed. Use `Metrics/LineLength: Max` instead

@marten marten changed the title [WIP] Allow users to edit and delete their account Allow users to edit and delete their account May 18, 2018
@marten marten requested a review from camallen May 18, 2018 10:26
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

.rubocop.yml: Lint/BlockAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/BlockAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/DefEndAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/EndAlignment has the wrong namespace - should be Layout
.rubocop.yml: Style/ConstantName has the wrong namespace - should be Naming
.rubocop.yml: Style/MethodName has the wrong namespace - should be Naming
.rubocop.yml: Style/PredicateName has the wrong namespace - should be Naming
.rubocop.yml: Style/FileName has the wrong namespace - should be Naming
.rubocop.yml: Style/AccessorMethodName has the wrong namespace - should be Naming
.rubocop.yml: Style/BlockLength has the wrong namespace - should be Metrics
Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in .rubocop.yml
`Style/IfUnlessModifier: MaxLineLength` has been removed. Use `Metrics/LineLength: Max` instead

Copy link
Contributor

@camallen camallen left a comment

Choose a reason for hiding this comment

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

LGTM - did you want to add a spec to ensure we sign the user out on the delete request? Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name)

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

.rubocop.yml: Lint/BlockAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/BlockAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/DefEndAlignment has the wrong namespace - should be Layout
.rubocop.yml: Lint/EndAlignment has the wrong namespace - should be Layout
.rubocop.yml: Style/ConstantName has the wrong namespace - should be Naming
.rubocop.yml: Style/MethodName has the wrong namespace - should be Naming
.rubocop.yml: Style/PredicateName has the wrong namespace - should be Naming
.rubocop.yml: Style/FileName has the wrong namespace - should be Naming
.rubocop.yml: Style/AccessorMethodName has the wrong namespace - should be Naming
.rubocop.yml: Style/BlockLength has the wrong namespace - should be Metrics
Error: obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in .rubocop.yml
`Style/IfUnlessModifier: MaxLineLength` has been removed. Use `Metrics/LineLength: Max` instead

@marten
Copy link
Contributor Author

marten commented May 18, 2018

@camallen Good catch. I've added a test for that, and also updated all of the other views such that #2410 will now be fixed with this as well. Might want to check it over again.

@marten
Copy link
Contributor Author

marten commented May 18, 2018

Oh and note that I'm renaming "sign up" to "create account" because it always confuses me (too close to "sign in")

@@ -0,0 +1,6 @@
root = true
Copy link
Contributor

Choose a reason for hiding this comment

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

does this file want to be in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it's some kind of standardized way of telling editors what settings to use for common things. Mine was indenting erb with 4 spaces, and figured this might be a good way of both fixing it and documenting it, even if people don't run the plugin. http://editorconfig.org/

Copy link
Contributor

Choose a reason for hiding this comment

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

sneaky

@marten marten merged commit ee4269c into master May 18, 2018
@marten marten deleted the delete-user-page branch May 18, 2018 14:04
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.

3 participants