foreman icon indicating copy to clipboard operation
foreman copied to clipboard

Fixes #38024 - Add ordering by virtual columns

Open ofedoren opened this issue 1 year ago • 2 comments

This is an alternative for https://github.com/theforeman/foreman/pull/10368, tries to make ordering by columns, which are not part of actual table, possible.

This PR enables it for roles model, making possible to click on locked column in roles index page and see the ordering, as well as doing so via API.

Added an entry for developer_docs saying how to use it within plugins.

Thanks, @adamruzicka, for the idea!

@MariaAga, what do you think?

P.S. The only issue I see is that we'd need to update https://github.com/theforeman/foreman-tasks/blob/master/app/controllers/foreman_tasks/api/tasks_controller.rb#L195 to be compatible. Could be done as part of removal deprecated stuff.

ofedoren avatar Nov 20 '24 15:11 ofedoren

It seems to work as expected in ui and api, the code makes sense but I think we need a Ruby person to verify it, @adamruzicka ?

MariaAga avatar Nov 21 '24 11:11 MariaAga

I've updated the logic a bit: since most of the UI controllers use resource_base_search_and_page/resource_base_with_search, I updated this method, so the new wrap_for_virt_column_select method is not necessary, but rather an optional helper. Although, looking at the changes in https://github.com/theforeman/foreman_openscap/pull/587, I don't think this wrapper will be used...

WDYT? Should we expose the wrapper or rather force usage of already defined loaders if ordering by virt columns desired?

ofedoren avatar Nov 29 '24 15:11 ofedoren

@adamruzicka, @MariaAga, reviving this one: do we still want that?

Note: it also fixes broken ordering by locked on roles page.

ofedoren avatar Jul 31 '25 13:07 ofedoren

/packit build

adamruzicka avatar Sep 30 '25 07:09 adamruzicka

Could you rebase please?

adamruzicka avatar Oct 02 '25 07:10 adamruzicka

Could you please squash the commits?

adamruzicka avatar Oct 02 '25 12:10 adamruzicka

Thanks, @adamruzicka, rebased / squashed. Although I'd like to see green tests just in case, but I don't think there will be related failures anyway :shrug:

ofedoren avatar Oct 02 '25 12:10 ofedoren

:green_apple:

ofedoren avatar Oct 02 '25 13:10 ofedoren

Thank you @ofedoren !

adamruzicka avatar Oct 02 '25 14:10 adamruzicka