feat: updated api to allow ordering on endorsed status #425
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #425 +/- ##
==========================================
+ Coverage 96.16% 96.19% +0.02%
==========================================
Files 58 58
Lines 4588 4624 +36
==========================================
+ Hits 4412 4448 +36
Misses 176 176 ☔ View full report in Codecov by Sentry. |
ayesha-waris
requested review from
AhtishamShahid,
saadyousafarbi and
muhammadadeeltajamul
| def check_thread_response_paging_json(thread, hash, resp_skip=0, resp_limit=nil, recursive=false, reverse_order=false) | ||
| check_thread_response_paging(thread, hash, resp_skip, resp_limit, true, recursive, reverse_order) | ||
| def check_thread_response_paging_json(thread, hash, resp_skip=0, resp_limit=nil, recursive=false, reverse_order=false, merge_question_type_responses=false) | ||
| check_thread_response_paging(thread, hash, resp_skip, resp_limit, true, recursive, reverse_order, merge_question_type_responses) |
There was a problem hiding this comment.
this new param passed is not taken as an input for the function check_thread_response_paging
There was a problem hiding this comment.
This parameter is intended to be used exclusively in the case of question-type threads. If provided, it serves to check whether the generated response matches that of a discussion-type thread."
| is_endorsed = num_comments > 0 && endorse_responses | ||
| hash = ThreadPresenter.new(thread, @reader, false, num_comments, is_endorsed, nil).to_hash(true, 0, default_resp_limit, true, false, true, true) | ||
| check_thread_result(@reader, thread, hash) | ||
| check_thread_response_paging(thread, hash, 0, default_resp_limit, false, false, true, true) |
There was a problem hiding this comment.
this is the same check we have for before this new param was added!
Can we be more specific in the test to check merge_question_type_responses params impact on the results returned! Right now, we are sort of only checking if we get a response or not
There was a problem hiding this comment.
check_thread_response_paging is handling merge_question_type_responses impacts on the results returned
| @@ -48,7 +49,8 @@ def to_hash( | |||
| end | |||
| sorting_key_order = reverse_order ? -1 : 1 | |||
| if with_responses | |||
| if @thread.thread_type.discussion? && resp_skip == 0 && resp_limit.nil? | |||
| if (@thread.thread_type.discussion? || (@thread.thread_type.question? && merge_question_type_responses)) && | |||
There was a problem hiding this comment.
what does this check mean now?
There was a problem hiding this comment.
The objective of this pull request (PR) is to ensure that the API responses for discussion type threads and question type threads are consistent. To achieve this, the check is to verify if the merge_question_type_responses parameter is present in the API call and if the thread type is a question. If both conditions are met, the API response for question type threads should match that of discussion type threads.
ayesha-waris
force-pushed
the
Ayesha/INF-1240
branch
from
9921392 to
9a1d9cc
Compare
INF-1240
Add new param to cs_comment service threads api to allow ordering on endorsed status