-
Notifications
You must be signed in to change notification settings - Fork 118
Add complete plan flag to V2 API #3595
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
base: api_v2_dmponline
Are you sure you want to change the base?
Conversation
- Edit plan query to check for user role to prevent eager loading - Add set_complete_param function that sets complete flag according to its value if its set to true in the API call
Generated by 🚫 Danger |
| if @complete | ||
| json.complete_plan do | ||
| q_and_a = presenter.complete_plan_data | ||
| next if q_and_a.blank? | ||
|
|
||
| json.array! q_and_a do |item| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Up to you, but this might be a tiny bit easier to read:
if @complete && presenter.complete_plan_data.present?
json.complete_plan do
json.array! presenter.complete_plan_data do |item|- Edit initialize to include complete flag, which is set to false as default - If flag is true in call, call fetch_all_q_and_a - Add fetch_all_q_and_a function that fetches questions and answers Update plan_presenter
20ba138 to
6261caf
Compare
- Add complete flag data to extension in json
d7b2d27 to
3771771
Compare
| next unless q.present? | ||
|
|
||
| { | ||
| title: "Question #{q.number || q.id}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we end up agreeing on the unique identifier (i.e. q.id) for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we agreed that it wasn't possible as is to have a unique identifier for a question that would remain unchanged if the question was edited, and that it was shippable as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, maybe I'm misunderstanding, but it might be possible.
When I create a plan, it has a plan.template associated with it. And that template has several questions associated with it. That template should always be associated with that plan, and those questions should always be associated with that template.
If an admin were to update a template question, a new template would be created. However, no plans with the prior template or prior questions would be affected.
Despite all that, we're running out of time. :P You can leave this to ship as-is for now if you'd like.
Changes proposed in this PR:
completeflag to the V2 API to enable accessing all questions and answers belonging to a plan in the API response.api/v2/plansandapi/v2/plans/[plan_id]endpoints. For example:api/v2/plans/12345?complete=true.