Defect #37626

Diff of a javascript file in repository module is not displayed with layout

Added by Trang Tran Thi Quynh about 1 month ago. Updated about 19 hours ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Marius BALTEANU% Done:

0%

Category:SCM
Target version:5.0.3
Resolution:Fixed Affected version:5.0.2

Description

At repository, when viewing differences of a javascript file, the revision page is showed with no styling as the image below

By the log content, no errors occurred. But I think request format JS is a mistake.

About my application's environment
Redmine 5
Ruby version              ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [x86_64-linux]
RubyGems version          3.3.7
Rails version             6.1.6
Active Record version     6.1.6
Action Pack version       6.1.6
Action Mailer version     6.1.6
Active Support version    6.1.6
Application root          /opt/redmine
Environment               production
Database adapter          mysql
Database schema version   5.5.41

error-1.png (151 KB) Trang Tran Thi Quynh, 2022-08-31 12:22

error-2.png (17.9 KB) Trang Tran Thi Quynh, 2022-08-31 12:22


Related issues

Related to Redmine - Defect #32449: Diff view for .js files in repositories is broken Closed
Related to Redmine - Defect #34984: Rails 6.1 Rendering actions with '.' in the name is depre... Closed

Associated revisions

Revision 21883
Added by Marius BALTEANU about 23 hours ago

Fix show diff of a javascript file in repository module is displayed without layout (#37626).

Patch by Trang Tran Thi Quynh.

Revision 21884
Added by Marius BALTEANU about 21 hours ago

Add to do to fix the deprecation warning (#37626).

Revision 21885
Added by Marius BALTEANU about 20 hours ago

Merge r21883-r21884 from trunk to 5.0-stable (#37626).

Revision 21886
Added by Marius BALTEANU about 19 hours ago

Fix rubocop warning (#37626).

Revision 21887
Added by Marius BALTEANU about 19 hours ago

Merge r21886 to 5.0-stable (#37626).

History

#1 Updated by Trang Tran Thi Quynh about 1 month ago

This bug was reported at https://www.redmine.org/issues/32449 and fixed. But it occurred again after fixed for Rails 6.1 Rendering actions with '.' in the name is deprecated #34984.

/app/controllers/repositories_controller.rb
-  render :diff, :formats => :html, :layout => 'base.html.erb'
+  render :diff, :formats => :html, :layout => 'base'

#2 Updated by Go MAEDA about 1 month ago

  • Category changed from Rails support to SCM
  • Status changed from New to Confirmed

#3 Updated by Go MAEDA about 1 month ago

  • Related to Defect #32449: Diff view for .js files in repositories is broken added

#4 Updated by Go MAEDA about 1 month ago

  • Related to Defect #34984: Rails 6.1 Rendering actions with '.' in the name is deprecated added

#5 Updated by Mizuki ISHIKAWA about 1 month ago

Rewriting to base.html.erb as before should solve the problem.
When displaying a diff of the js file, it appears to look for base.js if I don't explicitly write it.

diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb
index ff1230c883..56be9d526a 100644
--- a/app/controllers/repositories_controller.rb
+++ b/app/controllers/repositories_controller.rb
@@ -286,7 +286,7 @@ class RepositoriesController < ApplicationController
       @changeset = @repository.find_changeset_by_name(@rev)
       @changeset_to = @rev_to ? @repository.find_changeset_by_name(@rev_to) : nil
       @diff_format_revisions = @repository.diff_format_revisions(@changeset, @changeset_to)
-      render :diff, :formats => :html, :layout => 'base'
+      render :diff, :formats => :html, :layout => 'base.html.erb'
     end
   end

#6 Updated by Go MAEDA 28 days ago

Mizuki ISHIKAWA wrote:

Rewriting to base.html.erb as before should solve the problem.
When displaying a diff of the js file, it appears to look for base.js if I don't explicitly write it.

[...]

The patch fixes the issue but causes deprecation warnings.

DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated: layouts/base.html.erb (called from block in find_all at /Users/maeda/redmines/redmine-trunk/config/initializers/10-patches.rb:60)
DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated: layouts/base.html.erb (called from block in find_all at /Users/maeda/redmines/redmine-trunk/config/initializers/10-patches.rb:60)
DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated: layouts/base.html.erb (called from block in find_all at /Users/maeda/redmines/redmine-trunk/config/initializers/10-patches.rb:60)
DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated: layouts/base.html.erb (called from block in find_all at /Users/maeda/redmines/redmine-trunk/config/initializers/10-patches.rb:60)
DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated: layouts/base.html.erb (called from block in find_all at /Users/maeda/redmines/redmine-trunk/config/initializers/10-patches.rb:60)
DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated: layouts/base.html.erb (called from block in find_all at /Users/maeda/redmines/redmine-trunk/config/initializers/10-patches.rb:60)
DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated: layouts/base.html.erb (called from block in find_all at /Users/maeda/redmines/redmine-trunk/config/initializers/10-patches.rb:60)
  Rendering layout layouts/base.html.erb
  Rendering repositories/diff.html.erb within layouts/base.html.erb

#7 Updated by Go MAEDA 26 days ago

  • Target version set to Candidate for next minor release

Go MAEDA wrote:

The patch fixes the issue but causes deprecation warnings.

But I think we should prioritize fixing the problem when viewing JavaScript files than fixing deprecation warnings.

#8 Updated by Marius BALTEANU about 23 hours ago

  • Target version changed from Candidate for next minor release to 5.0.3

#9 Updated by Marius BALTEANU about 21 hours ago

  • Subject changed from At repository, viewing differences of a javascript file is lost style to Diff of a javascript file in repository module is not displayed with layout
  • Assignee set to Marius BALTEANU

#10 Updated by Marius BALTEANU about 19 hours ago

  • Status changed from Confirmed to Resolved
  • Resolution set to Fixed

I tried to add a functional test for this case, but without success, I think because the request made by minitest is different by the one made by the browser.

In the meantime, I've merged the proposed fix to 5.0-stable and I've opened a new issue (#37732) to properly fix this issue.

#11 Updated by Marius BALTEANU about 19 hours ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF