Functional tests for sorting on the /people page #9589

Manasa2850 · 2021-05-01T07:02:02Z

Partially Fixes #9411 (<=== Add issue number here)

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

PR is descriptively titled 📑 and links the original issue above 🔗
tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
code is in uniquely-named feature branch and has no merge conflicts 📁
screenshots/GIFs are attached 📎 in case of UI updation
ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

gitpod-io · 2021-05-01T07:02:04Z

codecov · 2021-05-01T07:09:08Z

Codecov Report

❗ No coverage uploaded for pull request base (main@c958118). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 886ebec differs from pull request most recent head 3cabaf8. Consider uploading reports for the commit 3cabaf8 to get more accurate results

@@           Coverage Diff           @@
##             main    #9589   +/-   ##
=======================================
  Coverage        ?   49.41%           
=======================================
  Files           ?       98           
  Lines           ?     6122           
  Branches        ?        0           
=======================================
  Hits            ?     3025           
  Misses          ?     3097           
  Partials        ?        0

Manasa2850 · 2021-05-01T14:07:04Z

@jywarren @cesswairimu some observations that I made from this PR.
The test for sorting the /people page based on last_activity fails saying Expected false to be truthy. The test passes if we have "i.updated_at" <= "j.updated_at" but this is not observed in the page i.e the results are not sorted based on ascending order of last_activity too. It's randomly displayed. Not sure why the test passes but the displayed results are different.

RuthNjeri · 2021-05-02T13:44:47Z

test/functional/users_controller_test.rb

+    get :list, params: { sort: 'last_activity' }
+
+    assert_response :success
+    assert assigns(:users).each_cons(2).all?{|i,j| "i.updated_at" >= "j.updated_at" }


Thanks for working on this @Manasa2850. With the test at line 316 and 323, should the signs >= be the inverse of each other? If alphabetical and ascending, then the i should be lower than the j? j.node_revisions.username" <= "i.node_revisions.username"

And for the last activity in descending order, it does make sense for the to be greater than j as you have written..

I haven't worked with that kind of comparison before, please let me know if what I am saying does not make sense.

@RuthNjeri since we want the username to be sorted in ascending order, I've written j.node_revisions.username" >= "i.node_revisions.username where j refers to the second entry and i to the first one.
In line 323 it is i.updated_at" >= "j.updated_at which is the same as j.updated_at" <= "i.updated_at and is the reverse of what is there in line 316.

Oh, I see, I think I missed the I and j difference, thanks for the clarification.

@Manasa2850 Maybe here we can try with i.node_revisions.maximum("timestamp")

Sure I'll try that out!

@Tlazypanda it shows the same result Expected false to be truthy and the test passes on interchanging the values of i and j.

hey @Manasa2850, I think removing the quotes around the comparison assert assigns(:users).each_cons(2).all?{|i,j| i.updated_at >= j.updated_at } might solve this, since its a time comparison. Thanks

Thanks @cesswairimu! That worked 🎉

great 🎉 🎉

RuthNjeri · 2021-05-07T18:47:15Z

app/controllers/users_controller.rb

@@ -106,7 +106,7 @@ def list
    if sort_param == 'username'
      order_string = 'username ASC'
    elsif sort_param == 'last_activity'
-      order_string = 'last_updated DESC'
+      order_string = 'updated_at DESC'


@Manasa2850 I am not too sure switching this to updated_at fixes the problem. The updated_at is related to the user details I think, and the last_updated is related to the user's activities.

I could be wrong but that's something you want to confirm, when is the last_updated changed and what changes affect the updated_at in Rails, the updated_at is a default column that updates the timestamp when the user details changes in the other columns which might be unrelated to the changes detected by the last_updated which might be looking at when a user was last active on the site.

@RuthNjeri I think you're right with respect to the updated_at column reflecting the timestamp when changes are made in other columns.
I don't find a column called last_updated in the User table so I'm not really sure how that's working.

@Manasa2850 they are fetching it in line 137 as an alias for the most recently updated node for that user 😅

Ohh I missed that. Thanks @Tlazypanda! 😃

If you need some input or more problem solving please reach out in chat! Thanks everyone for working together on this one!

codeclimate · 2021-06-27T17:56:06Z

Code Climate has analyzed commit 3cabaf8 and detected 0 issues on this pull request.

View more on Code Climate.

cesswairimu · 2021-06-28T16:02:36Z

test/functional/users_controller_test.rb

+    get :list, params: { sort: 'last_activity' }
+
+    assert_response :success
+    assert assigns(:users).each_cons(2).all?{|i,j| i.updated_at >= j.updated_at }


the test looks amiss though, because we want to order by node_revisions last updated right?
but this looks like its looking at users table last updated_at ..I will pull locally and try to find a work around and revert

Hey @cesswairimu @Manasa2850 any more activity here? please let me know how I can help 😅 I see the tests are now passing

oh, the test was a bit off, might need to rewrite... this skipped my mind...will check it out soon...thanks @Tlazypanda ❤️

I believe @Manasa2850 is overloaded a bit but will be back online very soon, so this can hold for a bit! Thanks!

stale · 2022-09-08T21:48:15Z

Hi 😄, this issue has been automatically marked as stale because it has not had recent activity. Don't worry you can continue to work on this and ask @publiclab/reviewers to add in-progress label 🎉 . Otherwise, it will be closed if no further activity occurs in 10 days -- but you can always re-open it if you like! 💯 Thank you for your contributions! 🙌 🎈.

test for sorting by username

7cf2168

test for last activity

2141b44

Manasa2850 requested a review from a team as a code owner May 1, 2021 13:35

RuthNjeri reviewed May 2, 2021

View reviewed changes

RuthNjeri reviewed May 7, 2021

View reviewed changes

Update users_controller_test.rb

3cabaf8

Manasa2850 closed this Jun 27, 2021

Manasa2850 reopened this Jun 27, 2021

Manasa2850 closed this Jun 27, 2021

Manasa2850 reopened this Jun 27, 2021

Manasa2850 requested a review from cesswairimu June 27, 2021 19:07

cesswairimu reviewed Jun 28, 2021

View reviewed changes

stale bot added the stale label Sep 8, 2022

stale bot closed this Sep 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Functional tests for sorting on the /people page #9589

Functional tests for sorting on the /people page #9589

Manasa2850 commented May 1, 2021 •

edited

Loading

gitpod-io bot commented May 1, 2021

codecov bot commented May 1, 2021 •

edited

Loading

Manasa2850 commented May 1, 2021

RuthNjeri May 2, 2021

Manasa2850 May 2, 2021

RuthNjeri May 7, 2021

Tlazypanda May 8, 2021

Manasa2850 May 8, 2021

Manasa2850 May 8, 2021

cesswairimu Jun 27, 2021 •

edited

Loading

Manasa2850 Jun 27, 2021

cesswairimu Jun 28, 2021

RuthNjeri May 7, 2021

Manasa2850 May 8, 2021

Tlazypanda May 8, 2021

Manasa2850 May 8, 2021

jywarren May 25, 2021

codeclimate bot commented Jun 27, 2021

cesswairimu Jun 28, 2021

Tlazypanda Jul 17, 2021

cesswairimu Aug 1, 2021

jywarren Aug 17, 2021

stale bot commented Sep 8, 2022

Functional tests for sorting on the /people page #9589

Functional tests for sorting on the /people page #9589

Conversation

Manasa2850 commented May 1, 2021 • edited Loading

gitpod-io bot commented May 1, 2021

codecov bot commented May 1, 2021 • edited Loading

Codecov Report

Manasa2850 commented May 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cesswairimu Jun 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot commented Jun 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Sep 8, 2022

Manasa2850 commented May 1, 2021 •

edited

Loading

codecov bot commented May 1, 2021 •

edited

Loading

cesswairimu Jun 27, 2021 •

edited

Loading