clear-code / redmine_full_text_search

Full text search for Redmine
MIT License
61 stars 24 forks source link

Enable sort function by registered time on the search page #119

Closed otegami closed 7 months ago

otegami commented 8 months ago

GitHub: fix #115 Related PR: https://github.com/clear-code/redmine_full_text_search/pull/118

What I did

Before After
Screenshot 2023-12-20 at 20 29 57 Screenshot 2023-12-29 at 9 20 53

What I checked

I checked whether the pgroonga command include registered_at in sort_key or not. And it does.

`'slices[type_filtered].sort_keys', '-registered_at'` - Steps - I access to the search page - http://localhost:3000/search - I sort the `News` by using registered time and check the logs rails generated. ```console SELECT pgroonga_command('select', ARRAY[ 'table', pgroonga_table_name('fts_targets_index_pgroonga'), 'match_columns', 'title * 100 || scorer_tf_at_most(content, 5)', 'query_flags', 'ALLOW_COLUMN|ALLOW_LEADING_NOT|QUERY_NO_SYNTAX_ERROR', 'filter', 'in_values(project_id, 1, 2, 3, 4, 5, 6) &! is_private == true &! ( in_values(project_id, 3, 4, 5, 6) && ( source_type_id == 6 || container_type_id == 6 ) ) && pgroonga_tuple_is_alive(ctid)', 'output_columns', '_id', 'limit', '0', 'drilldowns[source_type].keys', 'source_type_id', 'drilldowns[source_type].limit', '-1', 'drilldowns[container_type].keys', 'container_type_id', 'drilldowns[container_type].limit', '-1', 'slices[type_filtered].filter', 'all_records() &! source_type_id == 2 &! container_type_id == 2 &! source_type_id == 7 &! container_type_id == 7 &! source_type_id == 10 &! container_type_id == 10 &! source_type_id == 3 &! container_type_id == 3 &! source_type_id == 8 &! container_type_id == 8 &! source_type_id == 5 &! container_type_id == 5 &! source_type_id == 11 &! container_type_id == 11', 'slices[type_filtered].columns[highlighted_title].stage', 'output', 'slices[type_filtered].columns[highlighted_title].type', 'ShortText', 'slices[type_filtered].columns[highlighted_title].flags', 'COLUMN_SCALAR', 'slices[type_filtered].columns[highlighted_title].value', 'highlight_html(title)', 'slices[type_filtered].columns[content_snippets].stage', 'output', 'slices[type_filtered].columns[content_snippets].type', 'ShortText', 'slices[type_filtered].columns[content_snippets].flags', 'COLUMN_VECTOR', 'slices[type_filtered].columns[content_snippets].value', 'snippet_html(content)', 'slices[type_filtered].drilldowns[tag].keys', 'tag_ids', 'slices[type_filtered].drilldowns[tag].limit', '-1', 'slices[type_filtered].drilldowns[tag].sort_keys', '-_nsubrecs', 'slices[type_filtered].output_columns', '_score, content_snippets, id, last_modified_at, registered_at, project_id, source_id, source_type_id, title, highlighted_title, tag_ids', 'slices[type_filtered].sort_keys', '-registered_at', 'slices[type_filtered].offset', '0', 'slices[type_filtered].limit', '10' ] ) ```
otegami commented 7 months ago

@kou Thank you for reviewing. Could you give me the advice about the following comment?

kou commented 7 months ago

I'll re-review and reply after 2023-12-26...

otegami commented 7 months ago

@kou Thank you for reviewing. I've just fixed what you mentioned.

kou commented 7 months ago

Ah, the workflow is disabled by scheduled workflow. I've enabled it.

otegami commented 7 months ago

@kou Thank you for reviewing and enabling CI. I've just fixed what you mentioned.

otegami commented 7 months ago

@kou Thank you for reviewing. I've just fixed what you mentioned.

otegami commented 7 months ago

@kou Thank you for reviewing. I've just fixed what you mentioned.

kou commented 7 months ago

Could you update the PR title and description before we merge this? We use them for commit message.

otegami commented 7 months ago

@kou Sure. I've just updated the title and description.

otegami commented 7 months ago

Could you add a test for order by registered time?

fix: 7703bb5 @kou Sure, I've just added the test case for the order. Should I add the test case for the other orders like 'last_modified_time' or 'score'? If so, should I do it in the other issue?

kou commented 7 months ago

Should I add the test case for the other orders like 'last_modified_time' or 'score'?

It's better.

If so, should I do it in the other issue?

Yes.

otegami commented 7 months ago

@kou Thank you for reviewing. I've just fixed what you mentioned.


Should I add the test case for the other orders like 'last_modified_time' or 'score'?

It's better.

Added: I created the issue about it

kou commented 7 months ago

Thanks!