4948 - Fixing the sorting issues within parts of the app - part 1 of X #5299
Conversation
kellis5137
force-pushed
the
4948-fix-sort-issues-1
branch
from
da94875 to
5de4794
Compare
|
@dorner Could you take a look and let @kellis5137 know if there are any issues with their approach? Thanks! |
|
@kellis5137 I changed the "Resolves" to "Partial" above -- that way the workflow won't close the issue when this is merged. |
|
Oh, perfect!... should have done that
…On Sat, Aug 2, 2025 at 9:38 AM CL Fisher ***@***.***> wrote:
*cielf* left a comment (rubyforgood/human-essentials#5299)
<#5299 (comment)>
@kellis5137 <https://github.com/kellis5137> I changed the "Resolves" to
"Partial" above -- that way the workflow won't close the issue when this is
merged.
—
Reply to this email directly, view it on GitHub
<#5299 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOPZVVN6LQY52BGQSI2BUL3LS5K7AVCNFSM6AAAAACC45VMG2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNBWGUYDMMZYGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Hey @kellis5137 -- where does this show up in the UI? -- I signed in as superadmin and checked selecting an organization when adding a user, and it is in (Pawnee Diaper Bank, SF Diaper Bank, Second City Essentials Bank) order, whereas it should be in (Pawnee Diaper Bank, Second City Essentials Bank, SF Diaper Bank) order. Am I looking in the wrong place? |
Hmm.... I'll take a look, maybe I missed something |
@cielf It's so weird unless I'm looking at it wrong, but I've tried adding more resources in orders that should have thrown off the old code and still it's coming up in order. 
Is there another person that can give it a go? |
|
I'll try it again tomorrow -- it's possible that I had the wrong branch up. |
|
With 4948-fix-sort-issues 1 Signed in as superadmin@example.com I get: |
|
@kellis5137 Is it possible you haven't pushed your latest changes? |
|
Hmm... Well, I'm using GitHub codespaces... I'm going try a different
environment, pull my branch, and see what happens. Thanks for working with
this me... I'm guessing I messed up something, but I have no idea.I'll be
back!
…On Thu, Aug 7, 2025 at 1:27 PM CL Fisher ***@***.***> wrote:
*cielf* left a comment (rubyforgood/human-essentials#5299)
<#5299 (comment)>
@kellis5137 <https://github.com/kellis5137> Is it possible you haven't
pushed your latest changes?
—
Reply to this email directly, view it on GitHub
<#5299 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOPZVVNYWLMNWV4LR2YQZ33MOD6XAVCNFSM6AAAAACC45VMG2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNRVGEZDAMJQGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Oh - I'm not saying that's it -- I mostly do the functional testing, my Ruby's not advanced -- but/and on first glance, it looks to me like the code that's here would do ASCI sorting of the mixed case name, rather than of the lower-case. It's also possible that your environment works differently than mine. |
|
@cielf Oops, I think my problem is I was thinking this project was using MariaDB, not Postgres. They handle case sensitivity different. Try downloading the latest and let me know if it works. I also fixed the case insensitivity of the filter field as well. Give my latest a try. |
|
Question: Should this be sort by the number, the name, or does it matter? 
|
|
IIRC, that is currently by number, and should stay that way. Thanks for pointing it out! |
But it doesn’t seem to be sorted by either. |
|
Sorry -- It turns out I did not, indeed, remember correctly! I just double-checked production. The initial set are effectively sorted by name -- so I think that's the business's intent -- then later joiners are tacked on the end. So, let's make it by name. Thank you for your patience and sorry for the confusion |
@cielf I thought sent you a message, but I guess I didn't I believe the issue was I was thinking in terms of MySQL/MariaDB, but this project uses Postgres. Try downloading the latest and let me know if it's working now! |
|
@kellis5137 -- this PR does not include the sort for the NDBN memberships you asked the question about, right? |
|
Have also changed "resolves" to 'partial' - @kellis5137 for future -- please reserve "resolves" for PRs that totally resolve the issue, as our workflow closes the issue when we merge a PR that "resolves" an issue. |
|
No on the NDBN, that will be one of other things on the next PR. And I
apologize about the partial, I thought I changed that a while ago.
Keith
…On Thu, Aug 21, 2025 at 10:39 AM CL Fisher ***@***.***> wrote:
*cielf* left a comment (rubyforgood/human-essentials#5299)
<#5299 (comment)>
Have also changed "resolves" to 'partial' - @kellis5137
<https://github.com/kellis5137> for future -- please reserve "resolves"
for PRs that totally resolve the issue, as our workflow closes the issue
when we have "resolves".
—
Reply to this email directly, view it on GitHub
<#5299 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOPZVVTBMXHYMAEFX36M4D3OXKZZAVCNFSM6AAAAACC45VMG2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEMJQHA4DMNZQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
| end | ||
| end | ||
|
|
||
| describe "Resourse Dropdown List: Validate Order" do |
There was a problem hiding this comment.
Can we change this to a request spec? System specs are slow and flaky and should only be used if there needs to be browser / JavaScript interaction.
There was a problem hiding this comment.
@dorner Take a look now and let me know if that looks better
| # params | ||
| # - resource_type - Organization, Resource | ||
| # - q - query string | ||
| # visit admin_users_resource_ids_path # (name: "Organization", q: "") |
There was a problem hiding this comment.
This is pretty basic Rails info and I don't think we need a comment for it here.
There was a problem hiding this comment.
@dorner Take a look now and let me know if that looks better
kellis5137
force-pushed
the
4948-fix-sort-issues-1
branch
from
69f3fcd to
81b68ef
Compare
| it "Should sort display resource in human alphabetical order" do | ||
| get "/admin/users/resource_ids?resource_type=org_admin" | ||
| puts response.body | ||
| expect(response.body).to match(/{"results":\[{"id":\d+,"text":"Org ABC"},{"id":\d+,"text":"Pawnee"},{"id":\d+,"text":"Second City"},{"id":\d+,"text":"SF Diaper"}]}/) |
There was a problem hiding this comment.
If the response is JSON, it's much better to actually parse the JSON and use more specific matchers rather than regular expressions, which can be flakier.
kellis5137
force-pushed
the
4948-fix-sort-issues-1
branch
from
d863486 to
cb70bad
Compare
4 participants
Partial #4948
Description
This is a partial resolution to this problem. There is only one fix in this ticket, but it's been a while since I've worked with Ruby on Rails and, in particular, I want to make sure my spec looks good before I do other fixes.
The general problem is that some of the dropdown, lists, etc, are not being sorted in a human understandable alphabetical order. I am currently working through the app, looking for related issues.
No gems need to be updated/added.
Type of change
How Has This Been Tested?
via Unit test. Basically, when the data in the "resources_id" drop downs are displayed, they should be listed in case-insensitive alphabetical order.