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

Change paid to free in /resources #1284

Closed
aaron-junot opened this issue Oct 9, 2020 · 14 comments · Fixed by #1334
Closed

Change paid to free in /resources #1284

aaron-junot opened this issue Oct 9, 2020 · 14 comments · Fixed by #1334

Comments

@aaron-junot
Copy link
Member

Feature request

There is a pending change in the Resources API to change paid to free because it makes more sense as to what it means. See OperationCode/resources_api#379 When this change gets deployed, it will break /resources. We'd like to avoid that happening.

Proposed solution

This will be a two part solution. First, we need to change pages/resources/[page].js to send both paid and free in its requests so that way it will work with either. Once the API change happens, the front end should still be working. Then, we can go in and completely remove all references to paid on the front end. So those will be two different Pull Requests. The first will be deployed, then the Resources API will be deployed, then the second will be deployed.

Potential alternative solutions

Additional context

The ultimate idea is to have a checkbox that says free, and that should map directly to the api response of whether you have to pay for a resource or not. If free is true, then you know that you don't need to pay to access that resource. If free is false, then you must pay to access it.

@karmanya007
Copy link

So, where there is any thing related to paid, there should be a free one also?

@aaron-junot
Copy link
Member Author

paid is an attribute of a resource. Resources are either free or they are not. It's really just a simple change of the word. The tricky part, like I said, is making it backward compatible

@karmanya007
Copy link

Could you give me something to go off on?

@aaron-junot
Copy link
Member Author

Have you seen the page at https://operationcode.org/resources ?

That's the page that we're editing here. Maybe if you see the resources you'll understand what I'm talking about

@BhuwanChandra
Copy link
Contributor

@aaron-suarez can I work on this issue?

@aaron-junot
Copy link
Member Author

Sure @BhuwanChandra, go for it!

@BhuwanChandra
Copy link
Contributor

@aaron-suarez Ok, I am working on it.
do I need to make two PRs for this issue as you mentioned above?

@kylemh
Copy link
Member

kylemh commented Oct 12, 2020

I'm confused actually... @aaron-suarez I thought you said we wouldn't make it backwards compatible? I thought the plan was to make the breaking change on the back-end and then fix it here since the page isn't "released"

@aaron-junot
Copy link
Member Author

We had talked about both options, but I guess we thought different things about what we settled on 😅 . Either is fine with me. If we do break it, we'll want to deploy the related API change quickly, so let's wait to merge until I have some time to deploy right away.

We can just make it the one PR and break it, that's fine

@kylemh
Copy link
Member

kylemh commented Oct 12, 2020

👍 @BhuwanChandra you may want to wait until a deploy occurs, but then do not worry about backwards compatibility. Simply replace references to "paid" as "free" in the aforementioned file(s).

@aaron-junot
Copy link
Member Author

@kylemh since we're going to break it anyway, should we go ahead and merge+deploy OperationCode/resources_api#379 now?

@kylemh
Copy link
Member

kylemh commented Oct 12, 2020

yes

@kylemh kylemh added Hacktoberfest beginner friendly A relatively new developer should be able to accomplish this task Type: Feature/Redesign and removed beginner friendly A relatively new developer should be able to accomplish this task labels Oct 14, 2020
@BhuwanChandra
Copy link
Contributor

@aaron-suarez do I also need to make changes in UI as you mentioned in additional context?

@kylemh
Copy link
Member

kylemh commented Oct 15, 2020

No changes in the UI should be needed. We have a "resource cost" field that has a "paid" or "free" option. It's the VALUE those options represent that must change.

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

Successfully merging a pull request may close this issue.

4 participants