[Bug]: Form fields are lost if validation fails

r-ferrier commented 7 months ago

Description

input lost if validation fails

AC

[x] if validation fails edited fields maintain data.

notes

Ivan uncovered these fields that are failing

Partner

Slug / no
image upload (?)

Tags

Slug / no
Type (x) (disappears on failed save)
Assigned Users (disappears on failed save)

Site

Url / no

Calendar

partner organiser / no

Users

partners / no

~~We should investigate dynamic in-page form field validation.~~

~~From notion: https://www.notion.so/gfsc/Kim-s-timeline-for-completion-12-3-24-d0e8ef7a03c44ba08e56d4477e3fea5d Improve field validation process — can we have fields autosave as you go, etc? I think hotwire is meant to be good for this.~~

primary models

[x] tags/_form.html.erb
[x] partners/_form.html.erb
[x] sites/_form.html.erb
[x] calendars/_form.html.erb
[x] users/_form.html.erb

secondary models

[x] collections/_form.html.erb
[x] neighbourhoods/_form.html.erb
[x] supporters/_form.html.erb
[x] articles/_form.html.erb

kimadactyl commented 7 months ago

~~The biggest issue here is making a partner that already exists. You fill in all the fields to get a "partner already exists" notice. GI have hit this a few times.~~

~~Edit: maybe this is describing a slightly diff issue~~ -> https://github.com/geeksforsocialchange/PlaceCal/issues/2341

aaaaargZombies commented 7 months ago

That sounds like a different issue and a tricky one, we'd have to expose a list of existing partners to get around that. Long term that would look like transferring to a different security / permissions model. When I raised it before you proposed that GFSC staff deal with it manually. https://github.com/geeksforsocialchange/PlaceCal/issues/2063#issuecomment-1774955303

ivan-kocienski-gfsc commented 7 months ago

Research: multi-page activemodel saving

https://old.reddit.com/r/rails/comments/bxmo8i/whats_gems_if_any_do_you_use_to_build_complex/

Other thoughts:

Apart from what Alfie said there may also be a way to do this by having a partner have a "setup stage" that only shows a small subset of controls, validating only those controls as it goes, before saving completely and becoming "fully formed". Then the "new partner" interface would be a setup wizard with multiple steps.

But this would be a lot of work:

new field in partners
validations only run in blocks per page and how do we deal with validations that are cross-dependent i.e. needing an address and/or a neighbourhood (that is in the users neighbourhood pool)?
all the queries would need to be updated to discount partially complete partners (searching, filters, importing, admin interface)
new view for setup wizard with feedback, error feedback
what to do with partially complete partners?

aaaaargZombies commented 7 months ago

gonna watch this and see if we can provide validation before saving / losing info

https://www.youtube.com/watch?v=9zJ-r3Rsx7Q

Another option might be to trigger the models validation in the controller and then set the formfields in advance when resetting the page. We do something similar here.

    def validate_partner_relation
      # we only allow admins to assign partners they can see so any ids here are proof of a relationship
      return if current_user.root?
      return unless params[:user][:partner_ids].reject { |id| id == '' }.empty?

      # we're about to refresh the page so save the users input
      @user = User.new(permitted_attributes(User))
      flash.now[:danger] = 'User was not created: You must assign a Partner to the User or you will not be able to access it after creation'
      render 'new', status: :unprocessable_entity
    end

view code in context

aaaaargZombies commented 7 months ago

ok conclusion based on the video to do live feedback with hotwire

it looks quite straight forward, though there is a fair bit of manual work involved, putting in new routes and templates etc.
getting that sort of granular feedback and displaying it per input will be very time consuming.
perhaps showing the validations next to the submit button all together would be a sufficient compromise.

This is unlikely to solve the issue of dropped fields with an invalid submit but may reduce the amount people do that. We could probably disable the submit button based on this feedback until all fields are valid.

aaaaargZombies commented 7 months ago

I think I would give this ee with the basic implementation per form. If we are covering all forms or a form with a less basic implementation then the eee situation escalates quite quickly.

ivan-kocienski-gfsc commented 7 months ago

Checking this manually. So far I have checked

Partner

Name / yes
Slug / no
Summary / yes
description / yes
Accessibility Information / yes
image upload (?)
Street address / yes
Street address 2 / yes
Street address 3 / yes
City / yes
Postcode / yes
Service areas / yes
Website address / yes
Facebook link / yes
Twitter handle / yes
Instagram handle / yes
Public name / yes
Public email / yes
Public phone / yes
Partner name / yes
Partner email / yes
Partner phone / yes
Opening times / yes
Tags / yes

Tags

Name / yes
Slug / no
Description / yes
(system tag) / yes
Type (x) (disappears on failed save)
Tagged Partners / yes
Assigned Users (disappears on failed save)

Collection (has no validations)

Neighbourhoods (has only one validation for unit_code_value) (only root can edit)

Supporters (only root can modify supporters)

Name / yes (has validation)
URL / yes (no validation)
Description / yes
weight / yes

Articles

Title / yes
author / yes
body / yes
partners / yes
partnerships / yes
publish / yes

Site

Name / yes
Place name / yes
Tagline / yes
Url / no
Slug / yes
Hero text / yes
Description / yes
Published / yes
Site admin / yes
Theme / yes
Hero image credit / yes
hero alt text / yes
Main neighbourhood / yes
Badge zoom level / yes
other neighbourhoods to include / yes
partnerships / yes

Calendar

partner organiser / no
name / yes
url / yes
importer mode / yes
strategy / yes
default location / yes
public contact name / yes
public contact phone / yes
public contact email / yes

Users

first name / yes
last name / yes
email / yes
phone / yes
partners / no
neighbourhoods / yes
role / yes
partnerships / yes

ivan-kocienski-gfsc commented 7 months ago

Turns out the slug field is going to be tricky because we use this friendly_id gem that seems to want to do the slugging by itself but also we have the slug form field and they might be fighting each other.

kimadactyl commented 7 months ago

slug is root only, and tbh its kind of a pain letting people edit it as wrong stuff gets pasted in there all the time.

would be happy to move this to readonly as a stopgap if that helps things? if we are in a pinch we can always change it with console

site and tag creation is the only place i think we actually need people to be able to edit it? (hm perhaps this needs verifying a bit - its mostly partners that get junk data, and people messing with the slug on sites dont realise itll need a new ssl cert)

alternatively - lets just slap a big red warning on it - "dont edit if you dont know what youre doing" kind of thing - its only our few remaining root users this applies to anyway? (yeah this is prob easiest actually?)

geeksforsocialchange / PlaceCal