[5086] Rename executive director to contacts #5158

edwinthinks · 2025-04-20T03:25:28Z

Resolves #5086

Description

I can see that we wanted to change this from saying "Executive Director" since the field inside contains general contact information.

This includes some "scaffold" or temporary code that would be removed once migration goes out. For example, the executive director partials.

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Manually testing my migration and clicking through the application. I think this doesn't really need an additional system test since I think this isn't likely to break.

Screenshots

edwinthinks · 2025-04-20T03:27:23Z

...te/20250420031735_migrate_organization_partner_form_fields_executive_director_to_contacts.rb

@@ -0,0 +1,27 @@
+class MigrateOrganizationPartnerFormFieldsExecutiveDirectorToContacts < ActiveRecord::Migration[8.0]
+  class Organization < ApplicationRecord


Unlikely the Organization name would change, but this is a way to allow us to use the model name in the migration.

edwinthinks · 2025-04-20T03:27:53Z

app/helpers/partners_helper.rb

@@ -34,6 +34,11 @@ def partial_display_name(partial)
      'attached_documents' => 'Additional Documents'
    }

+    # TODO: temporary change of name for the contacts partial before migration


Once this is merged and deployed, we can remove these extra bits of code.

cielf · 2025-04-20T06:17:07Z

Hey @edwinthinks -- Thanks for taking this on! I'm busy with family obligations Sunday, so probably won't get to kicking the tires on this until at least Monday. I'll let @dorner make the call on tests, but I did notice that you haven't updated the user guide, and it looks like the linter is failing.

cielf

Hey @edwinthinks -- I see you've marked this WIP in the meantime, but I took a look anyway. Functionally it's close, but I did see a couple of things to adjust.

Summary

I think there's a mismatch between the change in the migration and how the organization view/edits pick up the list of partner profile edits, and that you missed changing the bank's partner view from executive director to contacts

Details/Recreation

For recreating the following, I started with the production branch, ran a fresh bin/setup, then picked Area Served, Executive Director, Pickup Person for the Partner Profile sections. Then I switched to this branch and ran the migration.

1/ "Contacts" is not showing up within the Partner Profile sections in the Organnization view -- but there's a blank entry where Executive Director was:

1a/ If I go into edit, "Contacts" is missing in the Partner Profile sections field

FWIW, I think it's stored -- just not showing up properly, because the partner profile functionality works at this point.

2/ The partner's view of their profile initially looks right, but the bank's view is missing the "Contacts" label:

3/ Next, on your branch, I selected "Area Served, "Contacts" and "Pickup Person" and saved the organization.
The bank's partner profile view now doesn't show the contacts -- I suspect it's still working off of executive_director. The rest seems to be working.

#Thoughts/theories on the weirdness

I'm wondering if a/ the migration removes "executive_director" from a string that contains a comma-separated list, leaving a ",," in it and b/ the organization view and edits' treatment of the Partner Profile section is sensitive to the order of the elements -- so it doesn't pick up on "Contacts" at the end of the list.

Anyway, If you solve those issues, and the user guide, we can pass it to @dorner for any technical input.

edwinthinks · 2025-05-03T22:12:16Z

Thanks for the review @cielf ; I'll take a look into addressing those.

edwinthinks · 2025-05-03T22:44:50Z

@cielf fixed everything except that thing you saw when you ran it in production. Any chance you can tell me what the output of that loop value is. In

human-essentials/app/views/organizations/_details.html.erb

Line 125 in fc95a66

<% for partner_form_field in @organization.partner_form_fields %>

                    <% for partner_form_field in @organization.partner_form_fields %>
                      <li>
                        <%= partner_form_field %>
                        <%= display_partner_fields_value(partner_form_field) %>
                      </li>
                    <% end %>

I'am looking at the migration and it shouldn't cause a empty ,, that seems strange.

cielf · 2025-05-04T01:37:44Z

Hey @edwinthinks -- Did you try the sequence I specified to recreate it? I have a busier than usual week this week, so I might not be able to get back to this until Friday.

edwinthinks · 2025-05-04T05:39:56Z

@cielf I don't believe I can since I don't have production data? And sure no rush :)

cielf · 2025-05-04T13:10:41Z

@edwinthinks For clarity, It's not production data -- it's the production branch (though the main branch should work as well). We don't have this situation in the default data, so we need to create it before running the migration to test it.

"For recreating the following, I started with the production branch, ran a fresh bin/setup, then picked Area Served, Executive Director, Pickup Person for the Partner Profile sections. Then I switched to this branch and ran the migration."

edwinthinks · 2025-05-04T22:03:55Z

For clarity, It's not production data -- it's the production branch

Ahh thanks, @cielf I was confused about that one. I'll try to replicate the issue then.

edwinthinks · 2025-05-05T00:17:12Z

Hey @cielf; I followed your steps to replicate the issue and was unable to get it what you were seeing. This would happen if you ran the migration and then went back to production branch. After migration are you staying on this branch? Maybe a restart might work?

Thanks!

cielf · 2025-05-05T18:34:23Z

It's plausible. I'll try to re-recreate.

cielf · 2025-05-06T15:22:34Z

Ok. That seems to be working. Sorry about the mixup.

cielf · 2025-05-06T15:25:01Z

Nit: The order on the view is not right after the migration, though it corrects itself the first time you save the organization.

cielf · 2025-05-06T15:47:56Z

@edwinthinks

It looks like the order of the partial names is a bigger problem than that, though.

By appending "Contacts" to the list, instead of replacing it in the same order, we end up changing the order of the partials in the partner profile edits, and the partner's view of their profile, though not in the bank's view of the partner's profile.

That's not a desirable side-effect - could be confusing to the users. And it would change back when the organization was saved (I'm pretty sure - haven't tested it.)

Is it workable to do the migration in a way that the order is preserved?
Alternatively, we would need to rework the partner profile edits, and the partner's profile view.

edwinthinks · 2025-05-07T09:56:03Z

Yep we can do it to retain the order for sure. Thanks @cielf

edwinthinks added 6 commits April 20, 2025 11:30

Fix step to migrating to contacts naming

4357d6c

More copy changes

5c3e06a

Rename Executive Director with contacts

4cff51a

Add a migration for the conversion to use the new field name

2869487

Update schema

1f67159

Remove some changes

af864e7

edwinthinks commented Apr 20, 2025

View reviewed changes

edwinthinks added the WIP label Apr 20, 2025

cielf self-requested a review April 21, 2025 16:55

cielf requested changes Apr 21, 2025

View reviewed changes

edwinthinks added 3 commits May 3, 2025 17:18

Merge branch 'main' into 5086/rename-executive-director

31f7212

Clean up

7707cce

Fix up guide

d3a2dbb

Fix linting erb errors

82e36af

		@@ -0,0 +1,27 @@
		class MigrateOrganizationPartnerFormFieldsExecutiveDirectorToContacts < ActiveRecord::Migration[8.0]
		class Organization < ApplicationRecord

Uh oh!

[5086] Rename executive director to contacts #5158

Are you sure you want to change the base?

[5086] Rename executive director to contacts #5158

Uh oh!

Conversation

edwinthinks commented Apr 20, 2025

Description

Type of change

How Has This Been Tested?

Screenshots

Uh oh!

edwinthinks Apr 20, 2025

Choose a reason for hiding this comment

Uh oh!

edwinthinks Apr 20, 2025

Choose a reason for hiding this comment

Uh oh!

cielf commented Apr 20, 2025

Uh oh!

cielf left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Summary

Details/Recreation

Uh oh!

edwinthinks commented May 3, 2025

Uh oh!

edwinthinks commented May 3, 2025

Uh oh!

cielf commented May 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edwinthinks commented May 4, 2025

Uh oh!

cielf commented May 4, 2025

Uh oh!

edwinthinks commented May 4, 2025

Uh oh!

edwinthinks commented May 5, 2025

Uh oh!

cielf commented May 5, 2025

Uh oh!

cielf commented May 6, 2025

Uh oh!

cielf commented May 6, 2025

Uh oh!

cielf commented May 6, 2025

Uh oh!

edwinthinks commented May 7, 2025

Uh oh!

Uh oh!

cielf left a comment •

edited

Loading

cielf commented May 4, 2025 •

edited

Loading