Skip to content

Implement medium priority changes related to Lead Delegate#14053

Open
maxidragon wants to merge 4 commits intothewca:mainfrom
maxidragon:lead-delegate-changes
Open

Implement medium priority changes related to Lead Delegate#14053
maxidragon wants to merge 4 commits intothewca:mainfrom
maxidragon:lead-delegate-changes

Conversation

@maxidragon
Copy link
Copy Markdown
Member

This PR implements medium priority changes from this document and one high priority change.

Summary:

Change the heading on the current “Delegates-All Regions” view for Senior and Regional Delegates. It currently says “Lead Delegate” and should be changed to “Senior and Regional Delegates”.

image

Add a tab to “Helpful Queries” that show key Delegate Statistics, specifically but not limited to the number of times a Delegate has been a “Lead Delegate”. This same information could also be available as a summary popup window on the “Regions” view of the Senior Delegate Panel

image image

Add the “Helpful Queries” view to the WEAT and WQAC panels.

WQAC already had it, so I added it only to the WEAT panel.

Add the number of times a delegate acted as a “Lead Delegate” to the CSV download for all delegates

To make this change possible I added a new field to roles_metadata_delegate_regions.


export default function DelegatesTable({
delegates, isAdminMode, isAllLeadDelegates, isAllNonLeadDelegates,
delegates, isAdminMode, isAllSeniorAndRegionalDelegates, isAllNonLeadDelegates,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should change the terminology of isAllNonLeadDelegates as well, I suppose


class AddLeadDelegatedCompetitionsToRolesMetadataDelegateRegions < ActiveRecord::Migration[8.1]
def change
add_column :roles_metadata_delegate_regions, :lead_delegated_competitions, :integer
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing column total_delegated does not have any _competitions suffix. You should stay consistent with that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also:

Suggested change
add_column :roles_metadata_delegate_regions, :lead_delegated_competitions, :integer
add_column :roles_metadata_delegate_regions, :lead_delegated_competitions, :integer, after: :total_delegated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really helpful for schema.rb because Rails sorts that file alphabetically, but useful for production where we want to keep column order deterministic (just for user convenience, no practical use)

Comment on lines +120 to +128
const { data: rolesData, isFetching: isFetchingRoles } = useQuery({
queryKey: ['hq-delegate-roles', userId],
queryFn: async () => {
const { data: d } = await fetchJsonOrError(
apiV0Urls.userRoles.list({ userId, groupType: groupTypes.delegate_regions }),
);
return d;
},
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... You're mixing two different sources of data here (your new roles metadata query and the existing fetchDelegated query), which are largely redundant.

This is a very subtle detail but: Your metadata computation uses actually_delegated_competitions, which filters for -> { over.visible.not_cancelled }. The "helpful queries" path for fetchDelegated uses delegated_competitions, which is basically the same relation but no filters at all (so in other words, even cancelled competitions are considered).

  1. Which of the two versions is the more desirable for WEAT?
  2. Can you modify the output of def delegated_competitions in users_controller.rb to give you all of the output data that you need to do lead delegate computations "on the fly", without needing to fire a second redundant query?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants