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

Fix User Edit Form bugs #235 #238

Closed
wants to merge 1 commit into from
Closed

Fix User Edit Form bugs #235 #238

wants to merge 1 commit into from

Conversation

@aditnryn
Copy link

@aditnryn aditnryn commented Apr 3, 2015

This PR aims to fix bullet 1 of #235. Please review the PR. I am not sure if this fix is the most elegent way to achieve it.

@aditnryn aditnryn changed the title Fix #235 Fix User Edit Form bugs #235 Apr 3, 2015
@wking
Copy link
Contributor

@wking wking commented Apr 3, 2015

On Fri, Apr 03, 2015 at 03:24:24PM -0700, Aditya Narayan wrote:

I am not sure if this fix is the most elegent way to achieve it.

Yeah, hitting the database to revert the unsaved changes doesn't seem
right, but I can't think of a better way either. Personally, I don't
think setting the title for the edit-page is worth an extra database
query, so I'd just drop this and go back to generic, model-level
titles (or just have the title be ‘amy’). If we want to keep the
entry-specific titles, this change is probably fine.

@aditnryn
Copy link
Author

@aditnryn aditnryn commented Apr 3, 2015

@wking Yes, I agree with you. I am against generating database query for changing the title. I would go for generic titles, unless there is another way to achieve the same without hitting the database.

@wking
Copy link
Contributor

@wking wking commented Apr 3, 2015

On Fri, Apr 03, 2015 at 03:50:50PM -0700, Aditya Narayan wrote:

I would go for generic titles, unless there is another way to
achieve the same without hitting the database.

I'm sure there is some way to cache the original values and then
restore the object if saving it fails, but I don't think per-object
titles matter enough to make it worth figuring out ;).

@gvwilson
Copy link
Contributor

@gvwilson gvwilson commented May 27, 2015

Are we expecting to merge this, or has it gone stale?

@pbanaszkiewicz
Copy link
Contributor

@pbanaszkiewicz pbanaszkiewicz commented May 27, 2015

I'll probably wontfix #235, but need some time to think about that issue. Should be this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants