Letter avatars still displayed after importing phpbb3 avatars


(Marguerite Su) #1

Hello,

Sorry for being new here.

I’m a former phpbb3 forum owner and I decided to import phpbb3 to discourse. The importer ran successfully. I even found a bug and it was fixed by our forum user, PR#5133 sent. I did enable all 3 options about avatars in settings.yml

After the import, I immediately launch sidekiq, puma and nginx to preview my new forum. And I noticed the avatars of my forum users imported are still lettered. I thought I should see their old avatars from the old forum. :sob: :sob: As the importer produced nothing wrong, I am confused.

Does it act like that because my sidekiq jobs are not finished? Or there’s anything deeper? I ran avatars:refresh and avatars:clean but no luck

Thanks for any hint! :heart_eyes: :heart_eyes:

Marguerite


phpBB to Discourse, Avatars are not displaying
(Jay Pfaffman) #2

It’s not sidekiq. My guess is that you didn’t get the importer configured correctly to find them.


(Marguerite Su) #3

@pfaffman

Thanks for your help!

avatars:
uploaded: true
gallery: true
remote: true

I got optipng, jhead, gifsicle and svgo installed correctly and can be found by discourse.

no error messages got, just the percentage in the log (I ran the importer with nohup so I can have any output saved)

And user avatars were copied, because I monkey patched the lib/cooked_post_processor.rb to show image’s absolute URL. and I saw user uploaded images in the posts were copied successfully from php_base_dir, so I best guess the avatars were also there in public/uploads. but I just could not see them applied. :sob: :sob:


(Marguerite Su) #4

BTW, attachments were imported and can be seen on the new forum. we have a forum magazine/summary produced in .pdf format. I can see its latest release on the new forum. So I guess php_base_dir was set up right.


(Marguerite Su) #5

can anyone see if it’s caused by this plugin:

in its plugin.rb, I saw this:

User.class_eval do
  def self.system_avatar_template(username)
    # TODO it may be worth caching this in a distributed cache, should be benched
    if SiteSetting.external_system_avatars_enabled
      url = SiteSetting.external_system_avatars_url.dup
      url.gsub! "{color}", letter_avatar_color(username.downcase)
      url.gsub! "{username}", username
      if username[0] =~ /[^\w]/
        url.gsub! "{first_letter}", (Pinyin.t(Romaji.kana2romaji(username)).strip.to_s[0] || '_').downcase
      else
        url.gsub! "{first_letter}", username[0].downcase
      end
      url
    else
      "#{Discourse.base_uri}/letter_avatar/#{username.downcase}/{size}/#{LetterAvatar.version}.png"
    end
  end
end

Does these codes override system functions and force usage of letter avatars?

Thanks.


(Gerhard Schlager) #6

Yes, I’m quite sure that this plugin is causing the problem. Why don’t you try without it…


(Marguerite Su) #7

@gerhard Hero! you save my life!

My forum is the semi-official forum for Chinese users of openSUSE Linux.

So it has lots of CJK usernames…this is the plugin commonly applied on discourse forums targeting on Chinese users.

I didn’t even think of such massive used plugin can be buggy like this…maybe their usages were:

  1. import
  2. apply plugin.

Mine was:

  1. apply plugin
  2. import

Thanks, I’ll try to fix that plugin


(Marguerite Su) #8

@gerhard @pfaffman

Sorry, guys. It wasn’t caused by the importer script nor the username-localization plugin.

  1. I uninstalled the plugin, dropped the database, and started over again. so the plugin problem can be avoided.

  2. After the import, I used pgweb to visit the database directly, and saw:

2.1 the user_avatar table has custom_upload_id field set.
2.1 the uploads table has my previous avatars

and I can visit the uploaded avatar, eg:

/uploads/default/original/1X/998dc371f7b0e6d21cbae0116072308e5e9e2dcd.png

it showed up.

But I can’t see it activated in posts, eg:

and the avatar in my profile is the default one too.

and the image url points to:

http://my_site/user_avatar/my_site/marguerite/120/2_1.png

which is also the default one.

Thanks for any help

Marguerite


(Marguerite Su) #9

@gerhard @darix

Hi,

After some debugging, I am afraid this is a bug.

Here’s how I did it.

The previous username localization plugin only overrides system_avatar_template, which is called by default_template, while default_template is called only when uploaded_avatar_id is not present.

I added some Rails logger and I can see uploaded_avatar_id printed.

If uploaded_avatar_id is present, it uses local_avatar_template, which is called by one function only: local_avatar_url.

After some grep, I found this: app/controllers/user_avatar_controller.rb

In its private part, there’s a ‘show_in_site’ function:

upload = Upload.find_by(id: upload_id) if user_avatar.contains_upload?(upload_id)
upload ||= user.uploaded_avatar if user.uploaded_avatar_id == upload_id

# mycode starts here:
# debug.txt is discourse:discourse owned.
open('/srv/www/vhosts/discourse/debug.txt', 'a+:UTF-8') do |f|
   f.write "user: #{username.downcase}," +
             "upload_id: #{upload_id}," +
             "upload: #{upload.url}, " +
             "contains: #{user_avatar.contains_upload?(upload_id)}, " +
             "uploaded_avatar_id: #{uploaded_avatar_id}," +
             "optimized: '#{get_optimized_image(upload, size)}'" +
             "\n"
end
# result => "marguerite, 36, xxx.png, true, 36, ''"
# mycode end:

if user.uploaded_avatar && !upload
elsif upload && optimized = get_optimized_image(upload, size)
   # should goes here, but not
   image
# there should an else with some logger error here, but no.
end

if image
else
  # image is nil, so goes here
  render_blank
end

so, I found upload exists, but optimized not exist. so render_blank (I think this is to use the default avatar).

I checked optimized_image table, it only contains a few (less than 10 compared to 4000+ users) optimized image.

So, this is the problem:

the importer script or discourse failed to create optimized image, but didn’t report anything.

Marguerite


(Darix) #10
$ rails.ruby2.4 c


User.all.map { |u| Jobs::CreateAvatarThumbnails.perform_async(upload_id: u.uploaded_avatar_id, user_id: u.id) if u.uploaded_avatar_id }

You should see something like:

[nil, "113db0ddae05b442464e73b6", "e7e4393a52970cb2cb3ebb57", "40886d7b9e5780119ac388f8", "320bcbbcbdf9c0feb791497c", "493e2c6ccf2b1fcf7ca15cc2"]

(Marguerite Su) #11

I ran it, and got outputs like that.

Does this add actions to sidekiq or run immediately?

Because I didn’t notice any change on the forum yet


(Marguerite Su) #12

@darix

Hi, my sidekiq jobs are finished, including the jobs scheduled by your pasted codes, but no luck.

After some debugging, I found this:

convert /srv/www/vhosts/discourse/public/uploads/default/original/1X/68af698194a44f88c6a9e058d392f60bb1b040f9.jpg[0] -auto-orient -gravity center -background transparent -thumbnail 75x75^ -extent 75x75 -interpolate bicubic -unsharp 2x0.5+0.7+0 -interlace none -quality 98 -profile /srv/www/vhosts/discourse/vendor/data/RT_sRGB.icm /tmp/discourse-test.jpg

convert: unrecognized interpolate method 'bicubic'

(Marguerite Su) #13

Changed ‘bicubic’ to ‘catrom’, it worked.

So I patched app/models/optimized_image.rb, and now everything works.

Thank you guys :wink:


Avatars not showing after import
phpBB to Discourse, Avatars are not displaying
(Darix) #14

@zogstrip should we have a setting for this? Or maybe have a test on startup which interpolation method works and setting the used option accordingly.

For the record the ImageMagick in question in 6.6.8. Funny our 7.0.6.7 binary doesnt support it either.

I just double checked the package. While parts of its documentation still mention the bicubic method. The header file with the struct definitely does not anymore. And it was not patched out in the package.

   InterpolateOptions[] =                                                                                                                                                     
   {                                                                                                                                                                          
     { "Undefined", UndefinedInterpolatePixel, UndefinedOptionFlag, MagickTrue },                                                                                             
     { "Average", AverageInterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                                
     { "Average4", AverageInterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                               
     { "Average9", Average9InterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                              
     { "Average16", Average16InterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                            
     { "Background", BackgroundInterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                          
     { "Bilinear", BilinearInterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                              
     { "Blend", BlendInterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                                    
     { "Catrom", CatromInterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                                  
     { "Integer", IntegerInterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                                
     { "Mesh", MeshInterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                                      
     { "Nearest", NearestInterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                                
     { "NearestNeighbor", NearestInterpolatePixel, UndefinedOptionFlag, MagickTrue },                                                                                         
     { "Spline", SplineInterpolatePixel, UndefinedOptionFlag, MagickFalse },                                                                                                  
 /*  { "Filter", FilterInterpolatePixel, UndefinedOptionFlag, MagickFalse }, */                                                                                               
     { (char *) NULL, UndefinedInterpolatePixel, UndefinedOptionFlag, MagickFalse }                                                                                           
   },  

Maybe it is worth to apply @marguerite 's change upstream?

More findings: It was renamed from Bicubic to Catrom. So definitely patching it would be a good idea. Still wondering if testing on startup which is supported should be done.


(Darix) #15

just curious did you have to rerun my loop after patching the optimized_image class?


(Marguerite Su) #16

Nope, I don’t need to, just restart discourse and avatars show.

For technical explanation, every time ‘show_in_site’ is called in the controller, it calls ‘get_optimized_image’, which refers to the OptimizedImage class and calls ‘create_for’ function. The later function contains return part if the thumbnail has already been there, so it is suitable to be ran every time. And if there was not, it will ‘resize’ or ‘crop’ the avatar (default to resize). And unluckily the default resize behavior failed because it didn’t check the return status of the command it wanted to run.

So unless the controller creates sidekiq jobs somewhere I didn’t notice, this change will take effect immediately :blush:


(Darix) #17

@zogstrip can we include this fix for the convert cmdline now? should i open a PR? (i will try without german comments this time. :stuck_out_tongue: )


(Régis Hanol) #18

Sure. Do you have an easy way to know what interpolation modes are available?