Skip to content

Fix autocompletion scoping with has_* through relations#233

Merged
adamruzicka merged 2 commits intowvanbergen:masterfrom
adamruzicka:has-through
Sep 30, 2025
Merged

Fix autocompletion scoping with has_* through relations#233
adamruzicka merged 2 commits intowvanbergen:masterfrom
adamruzicka:has-through

Conversation

@adamruzicka
Copy link
Collaborator

No description provided.

@adamruzicka adamruzicka marked this pull request as ready for review September 26, 2025 12:16
@adamruzicka
Copy link
Collaborator Author

Nope, hold on, not quite it

@adamruzicka
Copy link
Collaborator Author

This could do it, but it changes the autocomplete's behaviour a bit.

An example from Foreman - I have a Host with:

  • has_many :interfaces
  • has_one :primary_interface with where(primary: true) scope
  • has_one :domain, through: :primary_interface

Before this (and the previous scope change), calling Host.complete_for('domain = ') offered all domains in existence. After this change, it only offers domains, which are actually linked to a primary interface of a host.

Copy link

@ofedoren ofedoren left a comment

Choose a reason for hiding this comment

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

Thanks, @adamruzicka ! I've tested it a bit and it seems to work per description, also I've noticed that in the context of Foreman if I do Host.complete_for('location = ') it would suggest only locations which have a host. I'd say that's fine, but some might find it as a bug, what's your take on this?

Overall ACK from me.

@adamruzicka
Copy link
Collaborator Author

I'd say that's fine, but some might find it as a bug, what's your take on this?

I'd say having autocomplete offer you options which yield no results when evaluated is a bit pointless. So while it might be viewed as a bug, I'd say this new behaviour makes more sense.

@ofedoren
Copy link

I'd say that's fine, but some might find it as a bug, what's your take on this?

I'd say having autocomplete offer you options which yield no results when evaluated is a bit pointless. So while it might be viewed as a bug, I'd say this new behaviour makes more sense.

We should somehow propagate this take to the users of the library, like a heads up.

This is a hack. Some stale record was being left somewhere between the test
runs. Individual runs of auto_complete_spec and relation_querying_spec were
passing, but running both at once failed. Renaming a constant that was used in
both helped.
@adamruzicka
Copy link
Collaborator Author

I updated the changelog, once this goes in I'll update the wiki. Not sure if there's anything else that can be done

@adamruzicka adamruzicka merged commit 420e97f into wvanbergen:master Sep 30, 2025
12 of 18 checks passed
@adamruzicka adamruzicka deleted the has-through branch September 30, 2025 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants