plouc / mozaik-ext-github

Mozaïk github widgets
http://mozaik-github.herokuapp.com/
MIT License
11 stars 23 forks source link

Use lowest matching threshold message #4

Closed benediktvaldez closed 8 years ago

benediktvaldez commented 8 years ago

Currently we always get the message for the highest threshold, since it overwrites it everytime

benediktvaldez commented 8 years ago

@plouc have you checked this out? This is basically a bug :bug:

plouc commented 8 years ago

not yet, but if it's a bug, it deserves a test :)

benediktvaldez commented 8 years ago

Indeed it does :smile: There are no existing tests for PullRequestsGauge so I wasn't sure how you'd want to proceed.

The problem is that if you for example give the component the following

thresholds: [
    { threshold: 3,  color: '#85e985', message: 'good job!' },
    { threshold: 5,  color: '#ecc265', message: 'you should consider reviewing' },
    { threshold: 10, color: '#f26a3f', message: 'pull requests overflow' }
]

And the component goes through it like this

let message        = null;
let normThresholds = thresholds.map(threshold => {
    if (cappedValue <= threshold.threshold) {
        message = threshold.message;
    }

    return {
        upperBound: threshold.threshold,
        color:      threshold.color
    };
});

You will always get the pull requests overflow message, or basically the message for the highest threshold since it will always be the highest value compared to cappedValue

Fixing it is as simple as changing the if statement as follows

47 -  if (cappedValue <= threshold.threshold) {
47 +  if (message === null && cappedValue <= threshold.threshold) {