Patch #31994

Allow issue auto complete to return 10 issues when there is not search term provided

Added by Marius BALTEANU about 1 year ago. Updated about 1 year ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Issues
Target version:4.1.0

Description

The first patch changes the issue auto complete behaviour in order to return last ten updated issues when there is no search term provided. This will allow us to make future changes and propose to users some default issues in auto complete fields (one use case will be #31989) before they start typing a search term.

Second patch removes a view that is not used from my checks (code cleanup).

0002-Remove-app-views-auto_completes-issues.html.erb-whic.patch Magnifier (838 Bytes) Marius BALTEANU, 2019-09-02 07:13

0001-Issue-auto-complete-should-return-last-10-updated-is.patch Magnifier (2.53 KB) Marius BALTEANU, 2019-09-02 09:15

Related issues

Related to Redmine - Feature #31989: Inline issue auto complete (#) in fields with text-format... Closed

Associated revisions

Revision 18449
Added by Go MAEDA about 1 year ago

Issue auto complete should return last 10 issues (#31994).

Patch by Marius BALTEANU.

Revision 18450
Added by Go MAEDA about 1 year ago

Remove 'app/views/auto_completes/issues.html.erb' which is not used (#31994).

Patch by Marius BALTEANU.

Revision 18452
Added by Go MAEDA about 1 year ago

Fix the test name different from the actual behavior (#31994).

Patch by Marius BALTEANU.

History

#1 Updated by Go MAEDA about 1 year ago

  • Related to Feature #31989: Inline issue auto complete (#) in fields with text-formatting enabled added

#2 Updated by Go MAEDA about 1 year ago

What do you think if the patch checks params[:status] even when the search term is empty? I think it is more consistent.

diff --git a/app/controllers/auto_completes_controller.rb b/app/controllers/auto_completes_controller.rb
index f6a1414ad..bd8ce06b0 100644
--- a/app/controllers/auto_completes_controller.rb
+++ b/app/controllers/auto_completes_controller.rb
@@ -25,11 +25,10 @@ class AutoCompletesController < ApplicationController
     q = (params[:q] || params[:term]).to_s.strip
     status = params[:status].to_s
     issue_id = params[:issue_id].to_s
+
+    scope = Issue.cross_project_scope(@project, params[:scope]).visible
+    scope = scope.open(status == 'o') if status.present?
     if q.present?
-      scope = Issue.cross_project_scope(@project, params[:scope]).visible
-      if status.present?
-        scope = scope.open(status == 'o')
-      end
       if issue_id.present?
         scope = scope.where.not(:id => issue_id.to_i)
       end
@@ -39,6 +38,8 @@ class AutoCompletesController < ApplicationController

       issues += scope.like(q).order(:id => :desc).limit(10).to_a
       issues.compact!
+    else
+      issues += scope.order(:id => :desc).limit(10).to_a
     end

     render :json => format_issues_json(issues)

#3 Updated by Marius BALTEANU about 1 year ago

  • File deleted (0001-Issue-auto-complete-should-return-last-10-updated-is.patch)

#4 Updated by Marius BALTEANU about 1 year ago

Go MAEDA wrote:

What do you think if the patch checks params[:status] even when the search term is empty? I think it is more consistent.

Very good idea, thanks! I've updated the patch, also params[:issue_id] should be outside.

#5 Updated by Go MAEDA about 1 year ago

  • Target version set to Candidate for next major release

#6 Updated by Go MAEDA about 1 year ago

  • Target version changed from Candidate for next major release to 4.1.0

#7 Updated by Go MAEDA about 1 year ago

  • Assignee set to Marius BALTEANU

The subject says "return last 10 updated issues" but actually the patch returns last 10 created issues. Is it OK?

       issues += scope.like(q).order(:id => :desc).limit(10).to_a

#8 Updated by Marius BALTEANU about 1 year ago

  • Subject changed from Allow issue auto complete to return last 10 updated issues when there is not search term provided to Allow issue auto complete to return last 10 created issues when there is not search term provided
  • Assignee changed from Marius BALTEANU to Go MAEDA

Go MAEDA wrote:

The subject says "return last 10 updated issues" but actually the patch returns last 10 created issues. Is it OK?

[...]

Indeed, my initial intention was to return the last 10 updated issues, but during the implementation, I kept the existing behaviour. I'm updating the issue subject because changing from last created to last updated (which I still consider a good idea) should be discussed in another ticket. Go Maeda, thanks for catching this.

#9 Updated by Marius BALTEANU about 1 year ago

  • Subject changed from Allow issue auto complete to return last 10 created issues when there is not search term provided to Allow issue auto complete to return 10 issues when there is not search term provided

#10 Updated by Go MAEDA about 1 year ago

Marius BALTEANU wrote:

Second patch removes a view that is not used from my checks (code cleanup).

app/views/auto_completes/issues.html.erb is no longer used after r17881 (render :layout => false was replaced with render :json => format_issues_json(issues)).

#11 Updated by Go MAEDA about 1 year ago

  • Status changed from New to Closed

Committed the patches. Thank you for your contribution.

#12 Updated by Mischa The Evil about 1 year ago

Marius BALTEANU wrote:

Go MAEDA wrote:

The subject says "return last 10 updated issues" but actually the patch returns last 10 created issues. Is it OK?

[...]

Indeed, my initial intention was to return the last 10 updated issues, but during the implementation, I kept the existing behaviour.

Shouldn't this change be reflected in the name of the test that is committed (test_auto_complete_without_term_should_return_last_10_updated_issues)?

#13 Updated by Marius BALTEANU about 1 year ago

  • Status changed from Closed to Reopened

Mischa The Evil wrote:

Marius BALTEANU wrote:

Go MAEDA wrote:

The subject says "return last 10 updated issues" but actually the patch returns last 10 created issues. Is it OK?

[...]

Indeed, my initial intention was to return the last 10 updated issues, but during the implementation, I kept the existing behaviour.

Shouldn't this change be reflected in the name of the test that is committed (test_auto_complete_without_term_should_return_last_10_updated_issues)?

Indeed, it should, sorry for it.

Mariuss-MacBook-Pro:redmine mariusbalteanu$ git diff
diff --git a/test/functional/auto_completes_controller_test.rb b/test/functional/auto_completes_controller_test.rb
index 825ebf8e9..f8ed441da 100644
--- a/test/functional/auto_completes_controller_test.rb
+++ b/test/functional/auto_completes_controller_test.rb
@@ -151,7 +151,7 @@ class AutoCompletesControllerTest < Redmine::ControllerTest
     assert_include 'application/json', response.headers['Content-Type']
   end

-  def test_auto_complete_without_term_should_return_last_10_updated_issues
+  def test_auto_complete_without_term_should_return_last_10_issues
     # There are 9 issues generated by fixtures
     # and we need two more to test the 10 limit
     %w(1..2).each do

#14 Updated by Go MAEDA about 1 year ago

  • Status changed from Reopened to Closed

Mischa The Evil wrote:

Shouldn't this change be reflected in the name of the test that is committed (test_auto_complete_without_term_should_return_last_10_updated_issues)?

Thank you for pointing it out. Committed the fix posted in #31994#note-12 in r18452.

#15 Updated by Go MAEDA about 1 year ago

  • Related to Feature #32052: Auto-complete issues #id in search form added

#16 Updated by Go MAEDA about 1 year ago

  • Related to deleted (Feature #32052: Auto-complete issues #id in search form)

Also available in: Atom PDF