rubyforgood / homeward-tails

Homeward Tails is an application making it easy to link adopters/fosters with pets. We work with grassroots pet rescue organizations to understand how we can make the most impact.
MIT License
69 stars 117 forks source link

User only sees one error under input when form submit validation fails for edit registration #426

Closed kasugaijin closed 10 months ago

kasugaijin commented 10 months ago

Currently if you go to User > edit registration and make an error in the form e.g., password and submit, it renders an error at the top of the form AND under the input field. We only want an error under the input field (using bootstrap_form_for. This should also apply for an attachment e.g., if you try to attach a pdf file, an error should display under the attachment input.

atbalaji commented 10 months ago

I would like to work on this issue @kasugaijin

atbalaji commented 10 months ago

Hi @kasugaijin , few questions to clarify,

Shall I make a PR with only the removal change?

kasugaijin commented 10 months ago

Hi @atbalaji Yes please remove the top error only. As part of this issue, we should display the error below the input field for the attachment. As an example, enter a wrong password and submit the form and you will see the password field is highlighted in red with an error.

As for the pdf issue, I just merged a fix for that so please merge main into your branch and that should be resolved.

atbalaji commented 10 months ago

Ok @kasugaijin . I have a doubt here . This fixes the issue . But I am having a doubt of should I leave validates :append_avatar as it is or should i change this validation to validate and add custom methods and check all conditions in it since :append_avatar is a method. Before raising PR wanna know your thoughts

Screenshot 2024-01-11 010952

kasugaijin commented 10 months ago

@atbalaji I think it is OK to just use validates and set the conditions therein. It is also how we handle other attachments e.g. on Pet https://github.com/rubyforgood/pet-rescue/blob/main/app/models/pet.rb

atbalaji commented 10 months ago

Done and raised PR : https://github.com/rubyforgood/pet-rescue/pull/435

atbalaji commented 10 months ago

I'm pleased to inform that the pull request (#435 ) has been successfully merged. The changes have been incorporated, and everything looks good.

Considering this, could we kindly close this issue now?

kasugaijin commented 10 months ago

For sure @atbalaji thanks for the reminder.