Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updating ab chart stats TM-2409 #1909

Merged
merged 35 commits into from
Jan 21, 2022

Conversation

SidelineCory24
Copy link

@SidelineCory24 SidelineCory24 commented Jan 6, 2022

BE PR

To-Do

  • Bold percentages for Grade & Skill

@SidelineCory24 SidelineCory24 added do not merge WIP Work in progress labels Jan 6, 2022
@SidelineCory24 SidelineCory24 changed the title updating ab chart stats updating ab chart stats TM-2409 Jan 7, 2022
@SidelineCory24
Copy link
Author

SidelineCory24 commented Jan 12, 2022

@mjoyce91 when I click the "details" link for the codeclimate/diff-coverage I'm getting a 404 error.

…aPhase-Consulting/State-TalentMap into update/available-bidders-chart-stats
@mjoyce91
Copy link

@mjoyce91 when I click the "details" link for the codeclimate/diff-coverage I'm getting a 404 error.

@SidelineCory24 - just add an 'is defined' test for AvailableBidderStats.jsx

@mjoyce91
Copy link

Seeing some weird behavior on the percentages for these

Screen Shot 2022-01-18 at 1 07 49 PM

Screen Shot 2022-01-18 at 1 08 42 PM

@SidelineCory24
Copy link
Author

Seeing some weird behavior on the percentages for these

Screen Shot 2022-01-18 at 1 07 49 PM

Screen Shot 2022-01-18 at 1 08 42 PM

handling on BE

Comment on lines 30 to 31
const stats = get(biddersData, 'stats', {})[selectedStat] || [];
const statsSum = get(biddersData, 'stats.Sum', {})[selectedStat] || 0;

Choose a reason for hiding this comment

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

👍

@mjoyce91
Copy link

FE looks good, can approve once also working with back-end

@elizabeth-jimenez
Copy link

Seeing some weird behavior on the percentages for these

Screen Shot 2022-01-18 at 1 07 49 PM

Screen Shot 2022-01-18 at 1 08 42 PM

Good catch!

@elizabeth-jimenez
Copy link

elizabeth-jimenez commented Jan 18, 2022

image
we can remove the prepended Grade (would be on API side) NVM intentional

@SidelineCory24
Copy link
Author

Seeing some weird behavior on the percentages for these
Screen Shot 2022-01-18 at 1 07 49 PM
Screen Shot 2022-01-18 at 1 08 42 PM

Good catch!

this has been fixed on the BE

@SidelineCory24
Copy link
Author

SidelineCory24 commented Jan 19, 2022

image we can remove the prepended Grade (would be on API side) NVM intentional

image

@mjoyce91 @elizabeth-jimenez

Comment on lines 34 to 38
const grades$ = grades.map(grade => ({ ...grade, code: grade.name }));
grades$.sort(sortGrades);

const stat = selectedStat === 'Grade' ? grades$ : stats;

Choose a reason for hiding this comment

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

This may be cleaner, and easier to understand that stats are the statistics for a single section (opposed to all)

if (selectedStat === 'Grade') {
    stats = stats.map(grade => ({ ...grade, code: grade.name }));
    stats.sort(sortGrades);
  }

@mjoyce91 or is modifying stats a big no-no?

Choose a reason for hiding this comment

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

That's fine

Copy link
Author

Choose a reason for hiding this comment

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

done


// sorting grades to maintain consistency across the site
if (selectedStat === 'Grade') {
stats = stats.map(grade => ({ ...grade, code: grade.name }));

Choose a reason for hiding this comment

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

Nice job on making this an implicit return 🦜

@SidelineCory24 SidelineCory24 merged commit c46e4e8 into dev Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants