-
Notifications
You must be signed in to change notification settings - Fork 118
Add complete plan flag to V2 API #3592
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
Open
momo3404
wants to merge
12
commits into
api_v2_dmponline
Choose a base branch
from
momo/v2-api-qa-flag
base: api_v2_dmponline
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- This optional parameter will return all questions and answers in a plan when set to true
- Add the fetch_all_q_and_a function to return all questions and answers belonging to a plan to allow flag to work - Add complete_plan_data to call fetch function inside initialize
- Add call for complete flag - Add questions and answers of a plan to the json if complete is set to true
Generated by 🚫 Danger |
Replaced raw SQL with ActiveRecord query methods + enum scopes - Improves readability and eliminates SQL injection risk
Move theme filtering from Ruby to SQL using joins for better performance and scalability.
Previously, `fetch_q_and_a` iterated over all plan questions and searched for matching answers in Ruby, performing theme filtering and Q&A formatting in memory. This caused unnecessary loops, potential N+1 lookups, and inefficient handling of multi-theme questions. Accessing questions directly via `Plan.questions` also triggered joins through sections, phases, and templates, along with ordering (due to default scope), which added query overhead. The refactored version: - Queries answers directly via a join to question themes, filtering at the DB level. - Accesses questions through the answers (`answer.question`) instead of `plan.questions`, avoiding heavy joins and unnecessary default-scoped sorting.
Previously, the /api/v2/plans endpoint triggered many N+1 queries when rendering plan associations, causing slow response times. This change adds .includes() for all associations that were previously triggering Bullet N+1 warnings. Benchmark comparison (local dev, 10 sequential requests): | Metric | Before | After | | ----------------- | ------- | ------ | | Mean request time | 1909 ms | 492 ms | | Requests/sec | 0.52 | 2.03 | | Max request time | 2628 ms | 641 ms | - This simple comparison shows that these changes make the `GET api/v2/plans` ~4x faster.
- This optional parameter will return all questions and answers in a plan when set to true
- Add the fetch_all_q_and_a function to return all questions and answers belonging to a plan to allow flag to work - Add complete_plan_data to call fetch function inside initialize
- Add call for complete flag - Add questions and answers of a plan to the json if complete is set to true
…oadmap into momo/v2-api-qa-flag
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.