Skip to content
This repository was archived by the owner on Oct 13, 2025. It is now read-only.

[BD-38][INF-544][BB-6682] feat: implement reverse_order parameter - #398

Merged
asadazam93 merged 2 commits into
openedx-unsupported:masterfrom
open-craft:0x29a/bb6682/add_reverse_order_param
Nov 15, 2022
Merged

[BD-38][INF-544][BB-6682] feat: implement reverse_order parameter#398
asadazam93 merged 2 commits into
openedx-unsupported:masterfrom
open-craft:0x29a/bb6682/add_reverse_order_param

Conversation

@0x29a

@0x29a 0x29a commented Oct 31, 2022

Copy link
Copy Markdown
Contributor

Description

  • Added reverse_order query parameter to the threads endpoint.
  • Modified merge_response_content to support reversed content order.

Testing instructions

  1. Open Discussion for some course.
  2. Create a discussion, add two responses, add comments for both of them.
  3. Specify the thread ID (you can find it in the address bar) and course ID, and run the following (httpie used):
    http http://127.0.0.1:4567/api/v1/threads/<YOUR_THREAD_ID> "X-EDX-API-KEY: forumapikey" course_id==<YOUR_COURSE_ID> with_responses==True user_id==4 mark_as_read==False recursive==True resp_skip==0 reverse_order==false
    
    You should see a thread with comments ordered by the creation date.
  4. Run the command from the previous point, but change reverse_order==false to reverse_order==true. You should see threads and comments in the reversed order.
  5. You can specify resp_limit==1 to verify that the reverse ordering works with pagination.

@openedx-webhooks

openedx-webhooks commented Oct 31, 2022

Copy link
Copy Markdown

Thanks for the pull request, @0x29a!

When this pull request is ready, tag your edX technical lead.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 31, 2022
@0x29a 0x29a changed the title [WIP] feat: implement reverse_order parameter [WIP][BD-38][INF-544][BB-6682] feat: implement reverse_order parameter Oct 31, 2022
@openedx-webhooks openedx-webhooks added blended PR is managed through 2U's blended developmnt program and removed open-source-contribution PR author is not from Axim or 2U labels Oct 31, 2022
@0x29a
0x29a force-pushed the 0x29a/bb6682/add_reverse_order_param branch 3 times, most recently from 416c4c9 to 682c50a Compare October 31, 2022 06:01
@0x29a 0x29a changed the title [WIP][BD-38][INF-544][BB-6682] feat: implement reverse_order parameter [BD-38][INF-544][BB-6682] feat: implement reverse_order parameter Oct 31, 2022
Comment thread presenters/thread.rb
Comment on lines -130 to -132
if ancestry.empty? # invalid parent; ignore item
next
end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code does nothing.

@0x29a
0x29a force-pushed the 0x29a/bb6682/add_reverse_order_param branch from 682c50a to 8ded698 Compare October 31, 2022 07:37

@xitij2000 xitij2000 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Just one small comment. Looks good otherwise, and it seems to be working well.

  • I tested this: tested on devstack
  • I read through the code

Comment thread spec/presenters/thread_spec.rb
@xitij2000
xitij2000 force-pushed the 0x29a/bb6682/add_reverse_order_param branch from 5d39158 to 909b40a Compare November 8, 2022 07:40
@asadazam93

Copy link
Copy Markdown
Contributor

@0x29a can you please rebase this?

@0x29a
0x29a force-pushed the 0x29a/bb6682/add_reverse_order_param branch from 909b40a to e4c03a0 Compare November 8, 2022 11:23
@0x29a

0x29a commented Nov 8, 2022

Copy link
Copy Markdown
Contributor Author

@asadazam93, done.

@asadazam93
asadazam93 merged commit 063fd26 into openedx-unsupported:master Nov 15, 2022
@openedx-webhooks

Copy link
Copy Markdown

@0x29a 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@0x29a
0x29a deleted the 0x29a/bb6682/add_reverse_order_param branch November 15, 2022 12:24
ghassanmas pushed a commit to ghassanmas/cs_comments_service that referenced this pull request Dec 9, 2022
…82/add_reverse_order_param

[BD-38][INF-544][BB-6682] feat: implement reverse_order parameter
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

blended PR is managed through 2U's blended developmnt program

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants