direct sectors and affilated sectors#1184
Conversation
| </div> | ||
| </div> | ||
| <!-- Sectors --> | ||
| <% if @organization.profile_show_sectors? %> |
There was a problem hiding this comment.
not sure why this is so far outdented?
| .includes(person: :sectors) | ||
| .map(&:person) | ||
| .compact | ||
| .flat_map(&:sectors) |
There was a problem hiding this comment.
map and flat_map are slow, esp if there are a lot of affiliations (many of these orgs have hundreds of people). let's go with this for now, but it might be enough of a performance hit that we consider an alternative approach here and/or to lazy load on the organization show page.
|
looks great, @diti0-dot ! would you add a test to validate we see affiliated and direct sectors on org show and on populations served? |
|
@diti0-dot would you add a text link from org show to "Explore sector data" to get to this new page? |
|
@maebeale I’ve updated the indentation for the sectors section and renamed the link to point to /populations_served. Let me know if you’d prefer to open a separate issue for test or if I should continue on this PR |
|
@diti0-dot thank you! yes, please add the test as part of this PR. |
|
|
||
| <div class="mt-4"> | ||
| <%= link_to "Sector Distribution", | ||
| <%= link_to "Explore sector data", |
| <%= link_to "Sector Distribution", | ||
| <%= link_to "Explore sector data", | ||
| populations_served_organization_path(@organization), | ||
| class: "inline-block px-4 py-2 bg-gray-100 border border-gray-300 rounded shadow-sm text-gray-800 hover:bg-gray-200 transition" %> |
There was a problem hiding this comment.
would you update the styling to match the other text links at the top of the page (so it doesn't look like a button anymore)?
|
@diti0-dot we're deploying to production today, so i'm going to merge this. if you're still willing to create the test in a new pr, that'd be awesome! |
* direct sectors and affilated sectors * clean up indentation in sectors block * Update link style
2 participants
What is the goal of this PR and why is this important?
Show direct and affiliated sectors on the /populations_served page.
How did you approach the change?
Followed the given instructions and extended the existing populations_served implementation.
Anything else to add?
No additional changes.