intermix prefetched usernames and API responses, de-duplicating and c…

[TildaDares]

Part of larger planning issue #9667

Blending the list on the server side would be complex and inflexible and since both APIs return different JSON objects manipulating their objects would be a lot harder which is why I chose to do it on the client side.

No matter how I order the arrangement of both lists, the callback in the remoteFilter function always displays the usernames that match the first strings in the username at the top. In the video, you'll see this demonstrated when I type @a.

intermix.mov

You can test it here https://unstable.publiclab.org/

[gitpod-io]

[codecov]

Codecov Report

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

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

@@           Coverage Diff           @@
##             main    #9820   +/-   ##
=======================================
  Coverage        ?   26.03%           
=======================================
  Files           ?       98           
  Lines           ?     7620           
  Branches        ?        0           
=======================================
  Hits            ?     1984           
  Misses          ?     5636           
  Partials        ?        0           

[jywarren]

Blending the list on the server side would be complex and inflexible and since both APIs return different JSON objects manipulating their objects would be a lot harder which is why I chose to do it on the client side.

Great justification - awesome!

Looking now!

[jywarren]

This looks super!

No matter how I order the arrangement of both lists, the callback in the remoteFilter function always displays the usernames that match the first strings in the username at the top. In the video, you'll see this demonstrated when I type @A.

Do you think this is a problem? And if not, do you think this is ready to merge?

[jywarren]

The issue you mentioned about ordering is here:

So, are you saying there really isn't a way to intercept the ordering /after/ remoteFilter? If that's the case, i think it's OK. What do you think?

[jywarren]

Finally, is there a system test you'd like to add to this, which would protect the functionality? It could search for the recently active text, perhaps?

[jywarren]

And, it would be nice to have it in the same PR, so that the test and the functionality are linked in the history!

[TildaDares]

So, are you saying there really isn't a way to intercept the ordering /after/ remoteFilter? If that's the case, i think it's OK. What do you think?

I'm still looking through At.js docs and issues for how to override the remoteFilter. For now, I think the merge can wait.

[TildaDares]

Finally, is there a system test you'd like to add to this, which would protect the functionality? It could search for the recently active text, perhaps?

I'm definitely going to write a test. I just needed to confirm I was going in the right direction.

[TildaDares]

There's an open issue about the default filtering in ichord/At.js#448 but I've not found a solution. I tried changing the value of atwho_order to 0 but that didn't work. Looking at at.js code the ordering is determined by the index the query/string is found. I'm not sure I can override it.

[RuthNjeri]

Hi @TildaDares 👋🏾 , looking at this now, you can respond tomorrow.

What makes a user marked as recently active? should something be done with the users in the test case to make one active? Also, something else that could be an option is maybe adding a few seconds with before checking the assertion, with sleep or something else I have seen in the tests is a function called wait_for_ajax that I think waits for the response of an API call to complete...

[TildaDares]

@RuthNjeri I've tried the wait_for_ajax method and the sleep but it's not working. And a user is marked as active with the recently active text. Is there a way to look at the screenshots taken when running a system test? Locally, I can look at screenshots in the tmp/screenshots folder.

[codeclimate]

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

View more on Code Climate.

[RuthNjeri]

@RuthNjeri I've tried the wait_for_ajax method and the sleep but it's not working. And a user is marked as active with the recently active text. Is there a way to look at the screenshots taken when running a system test? Locally, I can look at screenshots in the tmp/screenshots folder.

Hi @TildaDares, I am not sure that there is a way to access the tmp/screenshot, currently I don't see any artifacts stored in the workflow

There is a process to enable it, maybe @jywarren can have a look at it? (the process requires access to the Repo settings), https://docs.github.com/en/github/administering-a-repository/managing-repository-settings/configuring-the-retention-period-for-github-actions-artifacts-and-logs-in-your-repository

I see you've updated the test to check for the list of names instead of the recently active text, that's okay too and a great alternative test. Maybe once we have access to the artifacts we can see what's happening...In the meantime, I'll have one final look and merge the PR 🚀

[jywarren]

Great problem solving and yes, i'll look at the artifacts! We never had time to get those working in GitHub Actions yet. It'd be great!

[jywarren]

OK, cool, i opened a PR for the screenshot saving here, let's see: #9868