Add section modal should check for errors #721

Closed
jerboaa opened this Issue Mar 14, 2012 · 9 comments

Comments

Projects
None yet
5 participants
@jerboaa
Member

jerboaa commented Mar 14, 2012

Steps to reproduce:

  1. Add new user (bring up add new user form)
  2. Click on the add new section button
  3. Enter name "test" and save
  4. Click add new section button again
  5. Enter name "test" and save

Actual result: To the user it seems that the section was saved, but it wasn't (sections need to be unique by name).

Expected result: Error message shown in the modal dialog (look at group invite dialog or role switch dialog how it's done). The modal dialog should not be closed in the error case.

@ghost ghost assigned Ayaya Mar 14, 2012

@jerboaa

This comment has been minimized.

Show comment
Hide comment
@jerboaa

jerboaa Mar 14, 2012

Member

@Ayaya could you have a look at this? Thanks!

Member

jerboaa commented Mar 14, 2012

@Ayaya could you have a look at this? Thanks!

@Ayaya

This comment has been minimized.

Show comment
Hide comment
@Ayaya

Ayaya Mar 14, 2012

Contributor

sure, I'll work on this.

Contributor

Ayaya commented Mar 14, 2012

sure, I'll work on this.

@Ayaya

This comment has been minimized.

Show comment
Hide comment
@Ayaya

Ayaya Mar 17, 2012

Contributor

Hi @jerboaa , I'm having issues getting the error to render properly within the modal dialog. I've been trying to follow the workflow from the role_switch dialog.. but I can't seem to get it to work.

So far I have it so:

  • when there is an error creating the section, I render a partial "add_new_section_handler" and pass the error message to it
  • the add_new_section_handler is an partial + rjs file which calls:
  • page.replace_html 'add_section_error', :partial => "student_form_add_section_error", :locals => { :error => error }
  • the student_form_add_section_error is simply just: div class="error"> <%= error %> </div

So when I try to run it, I get this rendered on the screen:

try { Element.update("add_section_error", "
Failed to create section.
"); } catch (e) { alert('RJS error:\n\n' + e.toString()); alert('Element.update("add_section_error", "
Failed to create section.
");'); throw e }

Any ideas?

Contributor

Ayaya commented Mar 17, 2012

Hi @jerboaa , I'm having issues getting the error to render properly within the modal dialog. I've been trying to follow the workflow from the role_switch dialog.. but I can't seem to get it to work.

So far I have it so:

  • when there is an error creating the section, I render a partial "add_new_section_handler" and pass the error message to it
  • the add_new_section_handler is an partial + rjs file which calls:
  • page.replace_html 'add_section_error', :partial => "student_form_add_section_error", :locals => { :error => error }
  • the student_form_add_section_error is simply just: div class="error"> <%= error %> </div

So when I try to run it, I get this rendered on the screen:

try { Element.update("add_section_error", "
Failed to create section.
"); } catch (e) { alert('RJS error:\n\n' + e.toString()); alert('Element.update("add_section_error", "
Failed to create section.
");'); throw e }

Any ideas?

@hoboman313

This comment has been minimized.

Show comment
Hide comment
@hoboman313

hoboman313 Mar 17, 2012

I think Severin is busy now...give me your branch # and I will take a look.

I think Severin is busy now...give me your branch # and I will take a look.

@Ayaya

This comment has been minimized.

Show comment
Hide comment
@Ayaya

Ayaya Mar 17, 2012

Contributor

Hey Egor, here is my branch:

https://github.com/Ayaya/Markus/tree/issue_721

Thanks,
Aaron

Contributor

Ayaya commented Mar 17, 2012

Hey Egor, here is my branch:

https://github.com/Ayaya/Markus/tree/issue_721

Thanks,
Aaron

@hoboman313

This comment has been minimized.

Show comment
Hide comment
@hoboman313

hoboman313 Mar 17, 2012

Note that I am not at my home computer, so I cannot actually test anything, but I see several problems with your code.

1.) I am guessing there is a page.replace_html for 'add_new_section_dialog' somewhere. In that case, these lines:

<script>
</script>

will be gone. Therefore, the code that you wrote in _add_new_section_handler.rjs will never actually work.

2.) First of all, the code that you added in _student_form_add_section_error.html.erb looks very generic and we might have something like this already. If not, I would consider putting it in views/shared.
Secondly, I don't think you understand AJAX in Rails 3. AJAX allows you to redraw a piece of a page without redrawing the entire page and in Rails 3 we tell forms to make AJAX calls by using ":remote => true". I see that the form that you created in _student_form_add_section_dialog.html.erb for #719 is a simple POST form. This is why you are only seeing the rendered view of:

<script>page.replace_html 'add_section_error', :partial => "student_form_add_section_error", :locals => { :error => error }</script>

in your response.
Adding ":remote => true" to your form will likely solve your problem.

Note that I am not at my home computer, so I cannot actually test anything, but I see several problems with your code.

1.) I am guessing there is a page.replace_html for 'add_new_section_dialog' somewhere. In that case, these lines:

<script>
</script>

will be gone. Therefore, the code that you wrote in _add_new_section_handler.rjs will never actually work.

2.) First of all, the code that you added in _student_form_add_section_error.html.erb looks very generic and we might have something like this already. If not, I would consider putting it in views/shared.
Secondly, I don't think you understand AJAX in Rails 3. AJAX allows you to redraw a piece of a page without redrawing the entire page and in Rails 3 we tell forms to make AJAX calls by using ":remote => true". I see that the form that you created in _student_form_add_section_dialog.html.erb for #719 is a simple POST form. This is why you are only seeing the rendered view of:

<script>page.replace_html 'add_section_error', :partial => "student_form_add_section_error", :locals => { :error => error }</script>

in your response.
Adding ":remote => true" to your form will likely solve your problem.

@Ayaya

This comment has been minimized.

Show comment
Hide comment
@Ayaya

Ayaya Mar 17, 2012

Contributor

Thanks Egor! That solved the issue! The error form was taken from the role_switch dialog error workflow, I think I will move it to views/shared and refactor the role_switch error view as well. Thanks for your tips!

Contributor

Ayaya commented Mar 17, 2012

Thanks Egor! That solved the issue! The error form was taken from the role_switch dialog error workflow, I think I will move it to views/shared and refactor the role_switch error view as well. Thanks for your tips!

@Ayaya

This comment has been minimized.

Show comment
Hide comment
@Ayaya

Ayaya Mar 17, 2012

Contributor

For the error view, I've decided to reuse the same one one that the original section add forms used which is already in views/shared. Review is in. Thanks!

Contributor

Ayaya commented Mar 17, 2012

For the error view, I've decided to reuse the same one one that the original section add forms used which is already in views/shared. Review is in. Thanks!

@mina

This comment has been minimized.

Show comment
Hide comment
@mina

mina Aug 13, 2012

Contributor

yikes please ignore the first commit (4ad7336)

Contributor

mina commented Aug 13, 2012

yikes please ignore the first commit (4ad7336)

benjaminvialle added a commit that referenced this issue Aug 14, 2012

Merge pull request #828 from mina/issue721
closes #721 - add section modal shows error if adding fails

benjaminvialle added a commit that referenced this issue May 19, 2013

Merge pull request #1085 from GhiGt/issue-726
Closes #721 : Error checking to the add section modal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment