nus-oss-test / testrepo4

TEAMMATES system is online at
http://teammatesv4.appspot.com
0 stars 0 forks source link

instructorStudentList: omit archived courses by default #1735

Closed damithc closed 10 years ago

damithc commented 10 years ago

From dam...@gmail.com on March 06, 2014 16:54:34

This will reduce the load on this page. But we should provide an option to include them as well.

Original issue: http://code.google.com/p/teammatespes/issues/detail?id=1686

damithc commented 10 years ago

From Gigi...@gmail.com on March 08, 2014 03:14:03

Code review: https://codereview.appspot.com/72860043/ Webpage Preview: http://xiekai.im/testArchivedDisplay.png

damithc commented 10 years ago

From Gigi...@gmail.com on March 08, 2014 03:21:39

my patch's idea:

  1. check a url parameter (displayArchive)
  2. if true, then the page will include archived courses; otherwise, they are removed in the page (not hidden, so load IS reduced).
  3. I use an additional checkbox to switch displaying or not
damithc commented 10 years ago

From dam...@gmail.com on March 08, 2014 04:25:34

James, can you review this?

Owner: Gigi...@gmail.com
Cc: jamesdju...@gmail.com
Labels: Reviewer-James

damithc commented 10 years ago

From Gigi...@gmail.com on March 08, 2014 20:02:16

Status: ReadyForReview

damithc commented 10 years ago

From jamesdju...@gmail.com on March 09, 2014 04:54:36

Sure Dr Damith, sorry for taking a bit long to reply.

From the page, the PageAction still actually request for all course data, including those archived. The modification mainly lies in the jsp where archived courses are not processed for display.

Is this what's intended Dr Damith? If so, then the patch is good.

damithc commented 10 years ago

From dam...@gmail.com on March 09, 2014 05:37:57

James, We can do this issue this way (it solves the problem partially) and create a new issue for reducing the load on the backend. It'll probably requires adding a new method to Logic API.

damithc commented 10 years ago

From jamesdju...@gmail.com on March 09, 2014 05:50:37

Alright, noted.

@Gigikie, we need a test for this change, probably on InstructorStudentListPageActionTest, UiTest.

Status: ChangesRequested

damithc commented 10 years ago

From Gigi...@gmail.com on March 09, 2014 06:53:35

noted

Status: Started

damithc commented 10 years ago

From Gigi...@gmail.com on March 09, 2014 23:40:17

code review: https://codereview.appspot.com/72860043/ added action test and ui test for preview patch

Status: ReadyForReview

damithc commented 10 years ago

From Gigi...@gmail.com on March 09, 2014 23:42:58

typo..

added action test and ui test for previous patch

damithc commented 10 years ago

From jamesdju...@gmail.com on March 11, 2014 07:43:45

Hi, just a few small changes, and it's good to merge

Status: ChangesRequested

damithc commented 10 years ago

From Gigi...@gmail.com on March 11, 2014 12:54:37

Code review: https://codereview.appspot.com/72860043/

Status: ReadyForReview

damithc commented 10 years ago

From jamesdju...@gmail.com on March 11, 2014 18:43:21

Status: ReadyToMerge

damithc commented 10 years ago

From arnold.k...@gmail.com on March 12, 2014 20:00:49

This issue was updated by revision 8aec0ca81365 .

Status: Delivered

damithc commented 10 years ago

From dam...@gmail.com on March 14, 2014 23:34:42

Status: Deployed
Labels: Milestone-V4.91