Patch #13688

Chosen thumbnail has to be bigger than requested one and not smaller

Added by Stanislav German-Evtushenko over 6 years ago. Updated 9 months ago.

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

0%

Category:Attachments
Target version:4.1.0

Description

Currently thumbnail request with a size of 99 will provide an image of 50px.
Currently thumbnail request with a size of 149 will provide an image of 100px.
And one exception: thumbnail request with a size of 49 and less will provide an image of 100px.

Provided image has to be bigger then requested and not smaller, for example:
request from 1 to 50 -> 50px
from 51 to 100 -> 100px
from 101 to 150 -> 150px
etc

small patch that makes this change is attached

attachment.rb.diff Magnifier (468 Bytes) Stanislav German-Evtushenko, 2013-04-04 21:31

0001-makes-sure-the-generated-thumbnail-is-always-of-at-l.patch Magnifier (1.77 KB) Jens Krämer, 2018-12-11 09:12

Associated revisions

Revision 17850
Added by Go MAEDA 9 months ago

Makes sure the generated thumbnail is always of at least the requested size (#13688).

Patch by Jens Krämer and Marius BALTEANU.

Revision 17851
Added by Go MAEDA 9 months ago

Fix a test failure (#13688).

Patch by Marius BALTEANU.

History

#1 Updated by Stanislav German-Evtushenko over 6 years ago

Before patch applied:
  • from 0 to 49 -> 100px
  • from 50 to 99 -> 50px
  • from 100 to 149 -> 100px
  • from 140 to 199 -> 150px
  • ...
After patch applied:
  • 0 -> 100px
  • from 1 to 50 -> 50px
  • from 51 to 100 -> 100px
  • from 101 to 150 -> 150px
    from 151 to 200 -> 200px
    ...

#2 Updated by Toshi MARUYAMA over 6 years ago

  • Category set to Attachments

#3 Updated by Jens Krämer 10 months ago

Here's an alternative patch using Float#ceil. Also adds a corresponding test case.

#4 Updated by Go MAEDA 10 months ago

  • Target version set to Candidate for next major release

#5 Updated by Go MAEDA 10 months ago

  • Status changed from New to Needs feedback

The following error occurred while running tests. Jens, could you check the patch?

Failure:
AttachmentsControllerTest#test_thumbnail_should_round_size [/home/ubuntu/redmine-trunk/app/models/attachment.rb:222]:
unexpected invocation: Redmine::Thumbnail.generate("/home/ubuntu/redmine-trunk/test/fixtures/files/2010/11/101123161450_testfile_1.png", "/home/ubuntu/redmine-trunk/tmp/thumbnails/16_8e0294de2441577c529f170b6fb8f638_300.thumb", 300)
unsatisfied expectations:
- expected exactly once, not yet invoked: Redmine::Thumbnail.generate()

bin/rails test test/functional/attachments_controller_test.rb:409

This is my environment:

Environment:
  Redmine version                4.0.0.devel.17734
  Ruby version                   2.6.0-p-1 (2018-12-06) [x86_64-linux]
  Rails version                  5.2.2
  Environment                    development
  Database adapter               SQLite
  Mailer queue                   ActiveJob::QueueAdapters::AsyncAdapter
  Mailer delivery                smtp
SCM:
  Subversion                     1.9.3
  Mercurial                      3.7.3
  Cvs                            1.12.13
  Bazaar                         2.7.0
  Git                            2.7.4
  Filesystem
Redmine plugins:
  no plugin installed

#6 Updated by Marius BALTEANU 9 months ago

Applying the below patch on top over Jens patch fixes the test.

diff --git a/test/unit/attachment_test.rb b/test/unit/attachment_test.rb
index 4bab579..bccb2c6 100644
--- a/test/unit/attachment_test.rb
+++ b/test/unit/attachment_test.rb
@@ -494,7 +494,7 @@ class AttachmentTest < ActiveSupport::TestCase
         [101, 150],
       ].each do |size, generated_size|
         thumbnail = attachment.thumbnail(size: size)
-        assert_equal "16_8e0294de2441577c529f170b6fb8f638_#{generated_size}.thumb",
+        assert_equal "8e0294de2441577c529f170b6fb8f638_2654_#{generated_size}.thumb",
           File.basename(thumbnail)
       end
     end
The thumbnail format is #{digest}_#{filesize}_#{size}.thumb where:
  • digest is 8e0294de2441577c529f170b6fb8f638
  • filesize is 2654
  • size is the generated size which changes during the test.

#7 Updated by Go MAEDA 9 months ago

  • Status changed from Needs feedback to New
  • Target version changed from Candidate for next major release to 4.1.0

Confirmed that the test passes after applying the patch written by Marius. Setting the target version to 4.1.0.

#8 Updated by Go MAEDA 9 months ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA

Committed the patch. Thank you for your contribution.

#9 Updated by Marius BALTEANU 9 months ago

  • Status changed from Closed to Reopened

To fix the tests:

diff --git a/test/functional/attachments_controller_test.rb b/test/functional/attachments_controller_test.rb
index cf7e961..faaf462 100644
--- a/test/functional/attachments_controller_test.rb
+++ b/test/functional/attachments_controller_test.rb
@@ -407,7 +407,7 @@ class AttachmentsControllerTest < Redmine::ControllerTest
     end

     def test_thumbnail_should_round_size
-      Redmine::Thumbnail.expects(:generate).with {|source, target, size| size == 250}
+      Redmine::Thumbnail.expects(:generate).with {|source, target, size| size == 300}

#10 Updated by Go MAEDA 9 months ago

  • Status changed from Reopened to Closed

Marius BALTEANU wrote:

To fix the tests:

[...]

Committed. Thanks.

Also available in: Atom PDF