fix: validate period flag

[k1nho]

Description

This PR adds a range validation to the insights contributors command. It will check for the value of the period flag. In case the period provided is invalid, it returns a descriptive error to signal that the API only accepts (7,30,90) as period values.

Before

$ pizza insights contributors open-sauced/app -p 45
Error: error while calling 'ContributorsServiceAPI.FindAllRecentPullRequestContributors' with repository 501028599': 400 Bad Request
error while calling 'ContributorsServiceAPI.FindAllRepeatPullRequestContributors' with repository 501028599: 400 Bad Request
error while calling 'ContributorsServiceAPI.FindAllChurnPullRequestContributors' with repository 501028599': 400 Bad Request
error while calling 'ContributorsServiceAPI.NewPullRequestContributors' with repository 501028599': 400 Bad Request

Usage:
  pizza insights contributors url... [flags]
...

Now

$ pizza insights contributors open-sauced/app -p 45
Error: invalid period: 45, accepts (7,30,90)

Usage:
  pizza insights contributors url... [flags]
...

What type of PR is this? (check all applicable)

• 🍕 Feature
• 🐛 Bug Fix
• 📝 Documentation Update
• 🎨 Style
• 🧑‍💻 Code Refactor
• 🔥 Performance Improvements
• ✅ Test
• 🤖 Build
• 🔁 CI
• 📦 Chore (Release)
• ⏩ Revert

Related Tickets & Documents

#66 --period flag broken

Mobile & Desktop Screenshots/Recordings

Added tests?

• 👍 yes
• 🙅 no, because they aren't needed
• 🙋 no, because I need help

Added to documentation?

• 📜 README.md
• 📓 docs.opensauced.pizza
• 🍕 dev.to/opensauced
• 📕 storybook
• 🙅 no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

[github-actions]

PR Compliance Checks

Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.

Issue Reference

In order to be considered for merging, the pull request description must refer to a specific issue number. This is described in our Contributing Guide.
This check is looking for a phrase similar to: "Fixes #XYZ" or "Resolves #XYZ" where XYZ is the issue number that this PR is meant to address.

[nickytonline]

Looks great @k1nho! One non-blocking suggestion.

[nickytonline]

refactor (non-blocking): No need for an explicit return of true or false unless this a preferred pattern in Go (I'm not a Gopher 😅)

Suggested change

	if period == 7 || period == 30 || period == 90 {
	return true
	}
	return false
	return period == 7 || period == 30 || period == 90

[k1nho]

Not a pattern, might be a side effect from too much if err != nil 😂

[jpmcb]

return period == 7 || period == 30 || period == 90

is valid Go and it really comes down to personal preference: there will be people who say this is a prime use case for a switch in Go in order to reduce the boolean mind games someone reading your code might need to do to figure out what's going on. Like:

switch range {
case 7:
	return true
case 30:
	return true
case 90:
	return true
default:
	// invalid range
	return false
}

but others would say that's too verbose and ends up being crufty 🤷🏼

Anywho, I'm good with what's written here and makes sense to me!

[k1nho]

there will be people who say this is a prime use case for a switch in Go in order to reduce the boolean mind games

true, there is also this way with the switch.

switch range {
case 7,30,90:
	return true
default:
	// invalid range
	return false
}

There is always the even-odd check meme way of doing it as well xD

[jpmcb]

Looks great! just one small suggestion!! Thanks for this - and thanks for your patience: I've been heads down on abunch of stuff the last few weeks!

[jpmcb]

Since this is a boolean function, can we rename this IsValidRange?

That typically reads better:

if !api.IsValidRange(opts.Period)

[k1nho]

thanks for the feedback!