hammerlab / cycledash

Variant Caller Analysis Dashboard and Data Management System
Other
35 stars 2 forks source link

Use username as a placeholder in the comment box if no other name is available #813

Closed armish closed 9 years ago

armish commented 9 years ago

If user is logged in, then we have a guess about what author name s/he would like to use when commenting. This change enables suggesting username as a default author name when commenting. If user replaces this with another custom name, then that becomes the default.

Here is how this behaves (note that I am logged in as Arman): screen recording 2015-08-20 at 06 15 pm

Review on Reviewable

hammer commented 9 years ago

Hmm why even make it editable if they have a username?

armish commented 9 years ago

I agree that they shouldn't be editable; but our current schema doesn't seem to allow that: https://github.com/hammerlab/cycledash/blob/master/schema.py#L47-L59

We ideally allow users to have a preferred display name to be used throughout the site and do not allow them to enter arbitrary text for author name when commenting. However, that requires a relatively more thorough refactoring and this is just a quick hack to encourage the use of usernames for now.

I will create a separate issue to improve the way we handle user <-> comment relationships.

ihodes commented 9 years ago

I think using the actually-logged-in user is actually an easier change that what you're having to do here. Instead of messing with the HTML/React code, you can just remove the Username field on the front-end, and on the backend just set the userId of the user who is commenting. This would require a migration (using Alembic, yay!)

armish commented 9 years ago

:+1:

Non-editable user ids and migration of the database, it is then! I will ditch this PR when I am done with the new one.

ihodes commented 9 years ago

Awesome, thanks! If you run into any troubles with the migration, let me know.

On Fri, Aug 21, 2015 at 11:29 PM, B. Arman Aksoy notifications@github.com wrote:

:+1:

Non-editable user ids and migration of the database, it is then! I will ditch this PR when I am done with the new one.

Reply to this email directly or view it on GitHub: https://github.com/hammerlab/cycledash/pull/813#issuecomment-133463396

armish commented 9 years ago

closing this PR -- #831 does a better job than this one.