[#4504] Add step-wise partner profile edit form #4766

danielabar · 2024-11-03T20:04:02Z

Resolves #4504

Description

This splits up the very long partner profile edit form into multiple collapsible sections, using the Bootstrap Accordion component. This project already uses Bootstrap so this does not introduce a new dependency.

The first attempt had a "Save and Next" button on each collapsible section, i.e. each one was a separate form that could be submitted individually. The original idea was to save one section and have the user returned to the edit view with the next section automatically opened for a kind of "guided" experience. It was also automatically saving work in progress edits as user collapsed/expanded sections. But this proved to be unworkable for a number of reasons. See #4504 comments for details.

So this solution simplifies things by only having a single form outside of the accordion section. User can click Save Progress which submits the entire form, or Submit for Approval. These action buttons appear both at the top and bottom of the accordion. If any validation errors occur, the user is returned to the edit view with all the sections that have error(s) expanded.

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

To exercise this feature, start by launching a Rails console bin/rails c and enable the new feature flag:

Flipper.enable(:partner_step_form)

Then with the Rails server running bin/start, navigate to http://localhost:3000, login as an Org admin, create a new Partner Agency (eg: [email protected]) and invite them. Then logout.

A new browser tab should open with the invitation email sent to the new partner. Click on the accept invitation button in this email, and create a password for the new partner. Once logged in, click "Edit my Profile" from the left navigation. You should now see the new step-wise form.

Expand and collapse as many sections as you'd like, and click Save Progress as often as you'd like.

Notice that the bootstrap open/close caret icons change to the word "Saving..." when the form is being submitted. This is to prevent the user from opening further sections and trying to edit in the middle of an update.

To see what the form looks like with multiple validation errors, fill it out as follows, then click Save Progress.

Media Information: Make all the social media links blank, and uncheck "No social media"
Pick up Person: Enter 4 comma separated emails for pick up email, eg: [email protected], [email protected], [email protected], [email protected]
Area served: Add some area(s) that don't add up to 100%
Partner Settings: Uncheck all the request settings

If the form renders with validation errors, then the Submit for Approval button should be disabled. Otherwise if partner is in Invited or Recertification status, then the Approval button is enabled. Otherwise the Approval button doesn't appear at all.

Also see system test for automated test: spec/system/partners/profile_edit_system_spec.rb

Optionally, disable the feature flag in Rails console Flipper.disable(:partner_step_form) and run through the invitation flow again (or use the same partner you already created). In this case, it should behave exactly as it does currently with the very large form.

Screenshots

Default rendering of step-wise edit form - all sections closed:

Multiple sections can be open/closed at once for editing:

Multiple sections open on validation errors:

- Introduce feature flag to control rendering of new partner step forms. - Update controller edit and update actions to use feature flag. - Add accordion UI for expand/collapse of sections in step forms. - Build out partner step forms for each section. - Update partner profile validation to set errors at field level. - Add service to map profile field errors to sections they belong in. - Update accordion styles to render as open if they contain an error. - Replace bootstrap caret icons with saving indicator during form submission. - Add system tests for partner profile step-wise editing.

dorner

Looks amazing! Please see a few questions/suggestions.

dorner · 2024-11-07T01:23:56Z

app/services/partners/section_error_service.rb

+  #   sections_with_errors = service.call
+  #   # => ["media_information", "pick_up_person", "partner_settings"]
+  #
+  class SectionErrorService


Is this basically an overgrown hash lookup? I feel like this can be squished into a single class-level method.

Updated with class method.

dorner · 2024-11-07T01:25:44Z

app/views/partners/profiles/step/edit.html.erb

+    <div class="accordion mx-5 mt-3" id="accordionExample">
+
+      <%# Agency Information %>
+      <div class="accordion-item">
+        <h2 class="accordion-header">
+          <button class="accordion-button <%= section_with_errors?('agency_information', @sections_with_errors) ? '' : 'collapsed' %>" type="button" data-bs-toggle="collapse" data-bs-target="#agency_information" aria-expanded="<%= section_with_errors?('agency_information', @sections_with_errors) %>" aria-controls="agency_information">
+            <div class='d-flex justify-content-between align-items-center w-100'>
+              <div class='fs-4'>
+                <i class="fa fa-edit mr-2"></i>Agency Information
+              </div>
+            </div>
+          </button>
+        </h2>
+        <div id="agency_information" class="accordion-collapse collapse <%= section_with_errors?('agency_information', @sections_with_errors) ? 'show' : '' %>">
+          <div class="accordion-body">
+            <%= render 'partners/profiles/step/agency_information_form', f: f, partner: current_partner, profile: current_partner.profile %>
+          </div>
+        </div>
+      </div>


Can this be extracted into a partial with relevant locals (name, partial ID etc.)? Partial renders can take blocks, so if you need specific HTML within the big skeleton you can do that if necessary.

Extracted to partial.

dorner · 2024-11-07T01:27:13Z

spec/models/partners/profile_spec.rb

+      end
+
+      it "sets error at field level when feature flag enabled for partner step form" do
+        allow(Flipper).to receive(:enabled?).with("partner_step_form").and_return(true)


Can you add the and_return(false) to the previous method? It's always best to call out explicitly when you depend on a value to be true/false rather than relying on defaults.

cielf · 2024-11-07T15:10:43Z

It's pretty nice! I need to think a bit about two things regarding a smooth passage through the process for the partners
1/ we might want to only check the social media when submitting for approval.
2/ It feels a bit off to make them save progress again when they have an error before they can try submitting for approval -- I know that in the current version, they have to have a clean save before submitting, but this is a different world.

…rror service

…t that expect errors at base level

danielabar · 2024-11-09T12:04:40Z

@dorner Technical feedback has been addressed.

@cielf Let me know if it should have further changes to the workflow.

cielf · 2024-11-09T21:47:20Z

Hey @danielabar -- I would like to make the change so that we don't disable the submit for approval button on error.
I also see, though, that on submit for approval with error, it's going to the view rather than returning to the edit -- it makes sense to me for it to return to the edit if there is an error.

I'm still pondering whether we should only check the social media when submitting for approval -- I don't like that you can't save current progress without doing it, but there would be a side effect of essentially not checking that section properly as it goes through -- and I haven't landed on which is less desirable.

dorner · 2024-11-09T23:32:08Z

app/views/partners/profiles/step/_accordion_section.html.erb

@@ -0,0 +1,24 @@
+<%# locals: (f:, partner:, section_id:, section_title:, icon_class:, partial_name:, sections_with_errors:) %>


dorner · 2024-11-09T23:32:36Z

Tech review passed. @cielf are there functional changes that have to happen?

cielf

I would like to make the change so that we don't disable the submit for approval button on error.
I also see, though, that on submit for approval with error, it's going to the view rather than returning to the edit -- it makes sense to me for it to return to the edit if there is an error.

danielabar · 2024-11-10T12:37:05Z

@cielf To date, I've only made the technical changes on this PR, not any workflow changes, since earlier comment mentioned it requires further thought.

The reason the Submit for Approval button is disabled on error state is from an earlier discussion on the issue. Don't recall which comment it was, but the idea was user shouldn't be able to Submit for Approval while form is in error. I could remove this logic if its no longer needed.

When form initially loads, it is considered valid because the social media validation runs only on edit, and the profile is created in an invalid state. That's why you can initially Submit for Approval, even though its guaranteed to generate the social media error.

Yes I see now that it goes to view rather than edit in this case, I'll look into that.

To confirm, I'll make the following changes:

Submit for Approval button is always enabled
Submit for Approval + error -> return to edit

cielf · 2024-11-10T12:54:48Z

That sounds right, with the caveat that on most forms we disable the save /submit buttons during the round-trip to the server so that we don't get double-clicks.

danielabar · 2024-11-10T13:11:13Z

@cielf Forgot to mention - for Submit for Approval, there is additional logic to only display it if parter is Invited or Recertification Required, otherwise it doesn't render at all. Is that still needed or should the button be displayed at all times?

Also fyi for this part "Submit for Approval + error -> return to edit", could be surprisingly thorny, requires modifications to the existing Approval Service which currently only seems to validate edit rules (i.e. only social media validation). I tried to change it (but should only change if step feature enabled), but strange things are happening. I still have to sort through it all but it will take more time.

I think the issue is the Approval Service wasn't intended as a full update service, but now as of this feature, it needs to do two things: Update the profile, and then request approval.

…h had modified the create service

danielabar · 2024-11-10T13:40:20Z

An overall thought/possible future issue (don't want to scope creep this one):

It seems like the complexity around social media validation and profile being invalid from the start is due to the existing PartnerCreateService which creates the profile in an invalid state:

Partners::Profile.create!({
                                    partner_id: @partner.id,
                                    name: @partner.name,
                                    enable_child_based_requests: organization.enable_child_based_requests,
                                    enable_individual_requests: organization.enable_individual_requests,
                                    enable_quantity_based_requests: organization.enable_quantity_based_requests
                                  })

If the above was modified to add:

no_social_media_presence: true

Then the profile would be valid right from the start, and it wouldn't require validate :check_social_media, on: :edit in the Profile model (could get rid of the on: :edit and other places like in Approval service where it needs to maintain awareness of edit state.

If the intention of creating a profile invalid from the beginning was to remind the user that they should fill in at least one social media field, perhaps this could be handled as a gentle reminder with some JS in the edit form, rather than creating a validation error right from the start.

… error state

cielf · 2024-11-10T14:06:09Z

"Then the profile would be valid right from the start, and it wouldn't require validate :check_social_media, on: :edit in the Profile model (could get rid of the on: :edit and other places like in Approval service where it needs to maintain awareness of edit state."

We'd still need it because they could uncheck it, or am I missing something?

I expect, though, that the intent was to make the partner think about it. Defaulting to none makes it easy to skip. Let me see if I can find the original ask. (or at least the issue for it)

danielabar · 2024-11-11T10:35:14Z

@cielf To clarify about the Submit for Approval. Currently it only validates the current state of the profile, but it's not a form submission, so if user made any other edits, they're not considered.

Should it also be doing a save in case user made further edits, then clicked the Submit for Approval without first clicking Save Progress?

i.e. it's really Save + Submit for Approval?

Note that combining these two separate services (Update Profile, Submit for Approval) is complicated so it would be good to confirm that's really the requirement.

@dorner Do you know if there's an example of a service that calls other services? I'm thinking of something like Organizer in the interactor gem, but for the custom service mixin that the existing services use.

cielf · 2024-11-11T13:23:10Z

(Thinking) We have two ways we could go here. 1/ Save and submit for approval. 2/ Save and Review - taking them to the view screen, where they would then submit for approval (matching the current flow).

Let me check in with one of our banks -- I believe we have a certain amount of partners not actually submitting for approval with the current model, but I want to confirm that.

(Edit. The bank I was going to consult with doesn't have time today. Maybe tomorrow.)

cielf · 2024-11-12T16:35:57Z

@danielabar Ok -- I've had a conversation with one of our main banks -- here's what we settled on:
1/ Change "Submit for Approval" to a "Save and Review", which will go to the Partner's profile view.
2/ On the partner profile view, twin the "Update Information" and "Submit for Approval" buttons to also be at the top

That gives the partners a final change to confirm that everything is right before submitting, but should also mitigate the problem of partners not actually submitting for approval. I can break out step 2 to be a separate issue if you want to keep it tight.

Is that workable?

…iew button

danielabar · 2024-11-13T11:35:16Z

@cielf I've changed:

"Submit for Approval" to "Save and Review". This new button performs the same logic as "Save Progress", but if the save is successful, then it redirects user to Profile view. I'm using the same flash success message "Details were successfully updated." for this case, let me know if it should be a different message?
Removed the conditional logic around displaying "Save and Review". Before it was only rendering for Invited and Recertification, but now that it simply saves and redirects user, I figure it can be run from any status, let me know if that's not the case?
Just like a previous discussion, the "Save and Review" button remains enabled, even when form is in an error state. User can always click it and if there are errors, they're taken back to edit view with error sections opened (just as if they had clicked "Save Progress"
Realized the Bootstrap primary color wasn't being applied to the "Save Progress" button, that's been fixed so it now looks like this:

I can look into the second part of the new requirement re: buttons at top and bottom of Partner Profile view.

danielabar · 2024-11-13T11:40:09Z

Observation, not sure if this is a bug or expected:

I can only save one Additional Documents attachment. i.e. if I return to the edit view after adding a document and add another one, the new one replaces the old one.

It seems to behave this way even with the feature flag disabled:

Flipper.enable(:partner_step_form)
# OR
Flipper.disable(:partner_step_form)

The Partner::Profile model does in theory support multiple attached documents:

has_many_attached :documents

But when I attach a new one, there's an ActiveStorage::PurgeJob running that removes the old one.

There may be an additional related bug that for the very first attachment, it doesn't get saved because the PurgeJob also runs. May require further investigation.

cielf · 2024-11-13T16:50:28Z

@danielabar I know there are some problems around Additional Documents in the current incarnation. it's out of scope for this PR. I'll see if I can find the appropriate issue -- the PurgeJob information may be new to it.

Oh -- you can add two documents at once -- just not add one, then add another one, but it's deleting the files if you edit again. Yup, that's no good.

Confirmed - we have a "proto issue" for this (i.e. its in our backlog to be written up), i'll throw the info about PurgeJob into it. I think we might need a little design thought to make this right.

cielf · 2024-11-13T17:03:05Z

@danielabar FYI, I'm probably not reviewing this today (severely underslept, and it needs a better brain than I have atm)

cielf · 2024-11-15T20:38:13Z

I'm pretty happy with this. Do I think we might want a tweak or two once we see how it flies? Yes. Do I think it's good to go public? Also yes -- though we'll probably hold off enabling it until we have the equivalent page for the bank.
@dorner Can you give a last tech look-over, since there have been a few additional changes, please?

dorner

Looks good to me!

dorner · 2024-11-16T23:28:29Z

@cielf is this good to merge?

cielf · 2024-11-17T00:54:24Z

@dorner Let me take a quick pass through with the flags off to make sure nothing broke.

github-actions · 2024-11-17T15:12:49Z

@danielabar: Your PR [#4504] Add step-wise partner profile edit form is part of today's Human Essentials production release: 2024.11.17.
Thank you very much for your contribution!

cielf requested review from cielf and dorner November 4, 2024 01:27

dorner requested changes Nov 7, 2024

View reviewed changes

danielabar added 3 commits November 9, 2024 06:15

[rubyforgood#4504] PR feedback - use class level method for section e…

d1e8bfd

…rror service

[rubyforgood#4504] PR feedback: specify feature flag disabled for tes…

1df9599

…t that expect errors at base level

[rubyforgood#4504] PR feedback: extract partial for accordion section

1d1ba35

dorner reviewed Nov 9, 2024

View reviewed changes

cielf requested changes Nov 10, 2024

View reviewed changes

undo what are now whitespace only changes from original solution whic…

df73809

…h had modified the create service

[rubyforgood#4504] PR feedback: remove approval disabled when form in…

f1c7438

… error state

[rubyforgood#4504] replace Submit for Approval link with Save and Rev…

1f94522

…iew button

dorner approved these changes Nov 16, 2024

View reviewed changes

cielf approved these changes Nov 17, 2024

View reviewed changes

cielf merged commit 11541b1 into rubyforgood:main Nov 17, 2024
12 checks passed

cielf mentioned this pull request Nov 17, 2024

Rework the bank's experience of the Partner Profile to match the partners' #4789

Closed

2 tasks

danielabar mentioned this pull request Nov 24, 2024

[#4789] add step-wise editing for banks view of partner profile #4799

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#4504] Add step-wise partner profile edit form #4766

[#4504] Add step-wise partner profile edit form #4766

danielabar commented Nov 3, 2024 •

edited

Loading

dorner left a comment

dorner Nov 7, 2024

danielabar Nov 9, 2024

dorner Nov 7, 2024

danielabar Nov 9, 2024

dorner Nov 7, 2024

danielabar Nov 9, 2024

cielf commented Nov 7, 2024

danielabar commented Nov 9, 2024

cielf commented Nov 9, 2024 •

edited

Loading

dorner Nov 9, 2024

dorner commented Nov 9, 2024

cielf left a comment

danielabar commented Nov 10, 2024

cielf commented Nov 10, 2024 •

edited

Loading

danielabar commented Nov 10, 2024 •

edited

Loading

danielabar commented Nov 10, 2024

cielf commented Nov 10, 2024

danielabar commented Nov 11, 2024 •

edited

Loading

cielf commented Nov 11, 2024 •

edited

Loading

cielf commented Nov 12, 2024

danielabar commented Nov 13, 2024

danielabar commented Nov 13, 2024

cielf commented Nov 13, 2024 •

edited

Loading

cielf commented Nov 13, 2024

cielf commented Nov 15, 2024

dorner left a comment

dorner commented Nov 16, 2024

cielf commented Nov 17, 2024

github-actions bot commented Nov 17, 2024

		@@ -0,0 +1,24 @@
		<%# locals: (f:, partner:, section_id:, section_title:, icon_class:, partial_name:, sections_with_errors:) %>

[#4504] Add step-wise partner profile edit form #4766

[#4504] Add step-wise partner profile edit form #4766

Conversation

danielabar commented Nov 3, 2024 • edited Loading

Description

Type of change

How Has This Been Tested?

Screenshots

dorner left a comment

Choose a reason for hiding this comment

dorner Nov 7, 2024

Choose a reason for hiding this comment

danielabar Nov 9, 2024

Choose a reason for hiding this comment

dorner Nov 7, 2024

Choose a reason for hiding this comment

danielabar Nov 9, 2024

Choose a reason for hiding this comment

dorner Nov 7, 2024

Choose a reason for hiding this comment

danielabar Nov 9, 2024

Choose a reason for hiding this comment

cielf commented Nov 7, 2024

danielabar commented Nov 9, 2024

cielf commented Nov 9, 2024 • edited Loading

dorner Nov 9, 2024

Choose a reason for hiding this comment

dorner commented Nov 9, 2024

cielf left a comment

Choose a reason for hiding this comment

danielabar commented Nov 10, 2024

cielf commented Nov 10, 2024 • edited Loading

danielabar commented Nov 10, 2024 • edited Loading

danielabar commented Nov 10, 2024

cielf commented Nov 10, 2024

danielabar commented Nov 11, 2024 • edited Loading

cielf commented Nov 11, 2024 • edited Loading

cielf commented Nov 12, 2024

danielabar commented Nov 13, 2024

danielabar commented Nov 13, 2024

cielf commented Nov 13, 2024 • edited Loading

cielf commented Nov 13, 2024

cielf commented Nov 15, 2024

dorner left a comment

Choose a reason for hiding this comment

dorner commented Nov 16, 2024

cielf commented Nov 17, 2024

github-actions bot commented Nov 17, 2024

danielabar commented Nov 3, 2024 •

edited

Loading

cielf commented Nov 9, 2024 •

edited

Loading

cielf commented Nov 10, 2024 •

edited

Loading

danielabar commented Nov 10, 2024 •

edited

Loading

danielabar commented Nov 11, 2024 •

edited

Loading

cielf commented Nov 11, 2024 •

edited

Loading

cielf commented Nov 13, 2024 •

edited

Loading