Merge pull request #1323 from ChrisBr/2.6

Release 2.6.7

ReleaseNotes-2.6.7

@@ -0,0 +1,37 @@
+#
+# openSUSE Build Service 2.6.7
+#
+
+Updaters from any OBS 2.6 release can just ugrade the packages
+and restart all services. Updaters from former releases should
+read the README.UPDATERS file.
+
+Feature backports:
+==================
+
+* none
+
+Changes:
+========
+
+* none
+
+Bugfixes:
+=========
+
+* [webui] drop hardcoded opensuse email adress and link
+* [webui] Make escape_project_list helper more generic
+* [webui] Remove default size from user_icon call
+* [webui] Remove unused user_icon code
+* [ci] Merge webui_helper_test files together
+* [ci][webui] Fix presentation of package file names in webui
+* [webui] Refactor owner search helpers
+* [webui] Refactoring arch_repo_table_cell
+* [webui] Replace ugly map with generic escape_nested_list helper
+* [webui] Refactor packages_table partial
+* [webui] Refactor project status page
+* [webui] Simplify user edit view
+* [webui] Refactor group index view
+* [ci] Improve webui_helper test#escape_nested_list
+* [webui] Remove unused html_safe call from build_logs view
+* [dist] Add 2.6.7 release notes

src/api/app/helpers/webui/package_helper.rb

@@ -50,8 +50,20 @@ def package_bread_crumb( *args )
   end
 
   def nbsp(text)
-    return text.gsub(' ', ' ')
-  end
+    result = "".html_safe
+    text.split(" ").each do |text_chunk|
+      result << text_chunk
+      result << "&nbsp;".html_safe
+    end
+    result.chomp!("&nbsp;")
 
+    if result.length >= 50
+      # Allow break line for very long file names
+      result = result.scan(/.{1,50}/).join("<wbr>")
+    end
+    # We just need to make it a SafeBuffer object again, after calling chomp and join.
+    # But at this point we know it truly is html safe
+    result.html_safe
+  end
 end
 

src/api/app/helpers/webui/search_helper.rb

@@ -5,7 +5,7 @@ module Webui::SearchHelper
   # @param [Symbol] type :user if the names are logins, :group if they are
   #         group names
   def search_owners_list(names, type = :user)
-    return "" if names.nil? || names.empty?
+    return [] if names.nil? || names.empty?
     output = []
     names.each do |role, list|
       if type == :group
@@ -14,6 +14,6 @@ def search_owners_list(names, type = :user)
         output += list.map {|user| user_and_role(user, role)}
       end
     end
-    output.join("<br />").html_safe
+    output
   end
 end

src/api/app/helpers/webui/webui_helper.rb

@@ -40,19 +40,8 @@ def user_icon(user, size=20, css_class=nil, alt=nil)
     user = User.find_by_login!(user) unless user.is_a? User
     alt ||= user.realname
     alt = user.login if alt.empty?
-    if size < 3 # TODO: needs more work, if the icon appears often on the page, it's cheaper to fetch it
-      content = user.gravatar_image(size)
-      if content == :none
-        content = Rails.cache.fetch('default_face') do
-          File.open(Rails.root.join('app', 'assets', 'images',
-                                    'default_face.png'), 'r').read
-        end
-      end
-      "<img src='data:image/jpeg;base64,#{Base64.encode64(content)}' width='#{size}' height='#{size}' alt='#{alt}' class='#{css_class}'/>".html_safe
-    else
-      image_tag(url_for(controller: :user, action: :icon, user: user.login, size: size),
-                width: size, height: size, alt: alt, class: css_class)
-    end
+    image_tag(url_for(controller: :user, action: :icon, icon: user.login, size: size),
+              width: size, height: size, alt: alt, class: css_class)
   end
 
   def fuzzy_time(time)
@@ -90,36 +79,49 @@ def status_id_for(repo, arch, package)
     valid_xml_id("id-#{package}_#{repo}_#{arch}")
   end
 
-  def arch_repo_table_cell(repo, arch, packname)
-    status = status_for(repo, arch, packname)
-    status_id = status_id_for(repo, arch, packname)
+  def arch_repo_table_cell(repo, arch, package_name)
+    status = status_for(repo, arch, package_name)
+    status_id = status_id_for(repo, arch, package_name)
     link_title = status['details']
     if status['code']
       code = status['code']
-      theclass='status_' + code.gsub(/[- ]/, '_')
+      theclass = 'status_' + code.gsub(/[- ]/, '_')
     else
       code = ''
-      theclass=''
+      theclass = ' '
     end
 
-    out = "<td class='#{theclass} buildstatus'>"
-    if %w(unresolvable blocked).include? code
-      out += link_to code, '#', title: link_title, id: status_id
-      content_for :ready_function do
-        "$('a##{status_id}').click(function() { alert('#{link_title.gsub(/'/, '\\\\\'')}'); return false; });\n".html_safe
+    result = "<td class='".html_safe
+    result += "#{theclass}"
+    result +=" buildstatus'>".html_safe
+
+    if %w(unresolvable blocked).include?(code)
+      result += link_to(code, '#', title: link_title, id: status_id, class: code)
+    elsif %w(- excluded).include?(code)
+      result += code
+    elsif @localpackages && !@localpackages.has_key?(package_name)
+      # Scheduled packages have no raw log file...
+      if 'scheduled' == code
+        result += code
+      else
+        result += link_to(code.gsub(/\s/, '&nbsp;'),
+                          raw_logfile_path(package: package_name,
+                                           project: @project.to_s,
+                                           arch: arch, repository: repo),
+                          title: link_title, rel: 'nofollow')
       end
-    elsif %w(- excluded).include? code
-      out += code
-    elsif @localpackages and not @localpackages.has_key? packname
-      out += link_to( code.gsub(/\s/, '&nbsp;'), raw_logfile_path(package: packname, project: @project.to_s, arch: arch, repository: repo),
-                                                 title: link_title, rel: 'nofollow')
     else
-      out += link_to code.gsub(/\s/, '&nbsp;'), { action: :live_build_log,
-                                                  package: packname, project: @project.to_s, arch: arch,
-                                                  controller: 'package', repository: repo }, { title: link_title, rel: 'nofollow' }
+      result += link_to(code.gsub(/\s/, '&nbsp;'),
+                        { action: :live_build_log,
+                          package: package_name, project: @project.to_s,
+                          arch: arch, controller: 'package', repository: repo
+                        },
+                        {
+                            title: link_title, rel: 'nofollow'
+                        })
     end
-    out += '</td>'
-    return out.html_safe
+    result += '</td>'.html_safe
+    result
   end
 
   REPO_STATUS_ICONS = {
@@ -394,26 +396,31 @@ def render_dialog(dialog_init = nil)
   # @param [String] user login of the user
   # @param [String] role title of the login
   # @param [Hash]   options boolean flags :short, :no_icon and :no_link
-  def user_and_role(user, role=nil, options = {})
+  def user_and_role(user, role = nil, options = {})
     opt = { short: false, no_icon: false, no_link: false }.merge(options)
-    realname = User.realname_for_login(user)
-    output = ''
+    real_name = User.realname_for_login(user)
 
-    output += user_icon(user) unless opt[:no_icon]
-    unless realname.empty? or opt[:short] == true
-      printed_name = realname + ' (' + user + ')'
+    if opt[:no_icon]
+      icon = ''
     else
-      printed_name = user
+      # user_icon returns an ActiveSupport::SafeBuffer and not a String
+      icon = user_icon(user)
     end
-    if role
-      printed_name += ' as ' + role
-    end
-    unless User.current.is_nobody?
-      output += link_to_if(!opt[:no_link], printed_name, user_show_path(user))
+
+    if !(real_name.empty? || opt[:short])
+      printed_name = "#{real_name} (#{user})"
     else
-      output += printed_name
+      printed_name = user
     end
-    output.html_safe
+
+    printed_name << " as #{role}" if role
+
+    # It's necessary to concat icon and $variable and don't use
+    # string interpolation! Otherwise we get a new string and
+    # not an ActiveSupport::SafeBuffer
+    User.current.is_nobody? ?
+        icon + printed_name :
+        icon + link_to_if(!opt[:no_link], printed_name, user_show_path(user))
   end
 
   def package_link(pack, opts = {})
@@ -530,16 +537,24 @@ def can_register
     return true
   end
 
-  def escape_project_list(projects)
-    # name and title are not html_safe
+  def escape_nested_list(list)
+    # The input list is not html_safe
     # because it's user input which we
     # should never trust!!!
-    projects.map { |project|
+    list.map { |item|
       "['".html_safe +
-      project[0] +
+      escape_javascript(item[0]) +
       "', '".html_safe +
-      escape_javascript(project[1]) +
+      escape_javascript(item[1]) +
       "']".html_safe
     }.join(",\n").html_safe
   end
+
+  def escape_list(list)
+    list.map { |p|
+      "['".html_safe +
+      escape_javascript(p) +
+      "']".html_safe
+    }.join(',').html_safe
+  end
 end

src/api/app/views/webui/configuration/groups.html.erb

@@ -22,8 +22,9 @@
             <tr id="group-<%= valid_xml_id(group.title) %>">
               <td><%= link_to(group.title, {:controller => 'group', :action => 'show', id: group.title}, {id: group.title}) %></td>
               <td class='users'>
-                <%= group.groups_users.map { |member|
-                  link_to(member.user, user_show_path(member.user)) }.join(', ').html_safe %>
+                <% group.groups_users.each_with_index do |member, index| %>
+                  <%= link_to(member.user, user_show_path(member.user)) %><%= ', ' if index < group.groups_users.size - 1 %>
+                <% end %>
               </td>
               <td class="nowrap">
                 <%= link_to(sprited_text('accessories-text-editor', 'Edit Group'), :controller => 'group', :action => 'edit', :group => group.title) %>

src/api/app/views/webui/home/index.html.erb

@@ -77,9 +77,8 @@
                                                              :project => 'REPLACEPRJ',
                                                              :package => 'REPLACEPKG') %> ">
       <% content_for :ready_function do %>
-        var ipackages = [ <%= @ipackages.map {|p|
-                           "['#{p[0]}','#{escape_javascript(p[1])}']" }.join(",\n").html_safe %> ];
-        renderPackagesProjectsTable({packages: ipackages, length: '15', name: 'ipackages_wrapper'});
+        var ipackages = [ <%= escape_nested_list(@ipackages) %> ];
+        renderPackagesProjectsTable({packages: ipackages, length: 15, name: 'ipackages_wrapper'});
       <% end %>
     </div>
     <% else %>
@@ -93,8 +92,7 @@
     <% else %>
     <div id="projects_table_wrapper" data-url="<%= url_for(controller: 'project', action: 'show', project: 'REPLACEIT') %>">
     <% content_for :head_javascript do %>
-      var main_projects = [ <%= @iprojects.map {|p|
-                           "['#{p[0]}','#{escape_javascript(p[1])}']" }.join(",\n").html_safe %> ];
+      var main_projects = [ <%= escape_nested_list(@iprojects) %> ];
       var excl_projects = [];
       <% end %>
       <% content_for :ready_function do %>
@@ -114,9 +112,8 @@
                                                              :project => 'REPLACEPRJ',
                                                              :package => 'REPLACEPKG') %> ">
           <% content_for :ready_function do %>
-            var iowned = [ <%= @owned.map {|p|
-                                "['#{p[1]}','#{escape_javascript(p[0])}']" }.join(",\n").html_safe %> ];
-            renderPackagesProjectsTable({packages: iowned, length: '15', name: 'iowned_wrapper'});
+            var iowned = [ <%= escape_nested_list(@owned) %> ];
+            renderPackagesProjectsTable({packages: iowned, length: 15, name: 'iowned_wrapper'});
           <% end %>
         </div>
       <% end %>

src/api/app/views/webui/package/update_build_log.js.erb

@@ -1,10 +1,10 @@
 <% if @finished %>
-  $('#log_space').append('<%= escape_javascript(@log_chunk.html_safe) %>');
+  $('#log_space').append('<%= escape_javascript(@log_chunk) %>');
   build_finished();
   hide_abort();
 <% else %>
   show_abort();
-  $('#log_space').append('<%= escape_javascript(@log_chunk.html_safe) %>');
+  $('#log_space').append('<%= escape_javascript(@log_chunk) %>');
 <% if @log_chunk.length < @maxsize || @initial == 0 %>
   autoscroll();
   setTimeout(function() { refresh(<%= @offset %>, 0); },2000);

src/api/app/views/webui/project/_packages_table.html.erb

@@ -24,7 +24,7 @@
           <% cache [packageurl, array_cachekey(@packages)] do %>
               <div id="packages_table_wrapper" data-url="<%= packageurl %>">
                 <%= javascript_tag do %>
-                    var packages = [ <%= @packages.map { |p| "['#{p}']" }.join(',').html_safe %> ];
+                    var packages = [ <%= escape_list(@packages) %> ];
                     renderPackagesTable("packages_table_wrapper", packages);
                 <% end %>
               </div>
@@ -53,8 +53,8 @@
           <% cache [packageurl, array_cachekey(@ipackages)] do %>
               <div id="ipackages_wrapper" data-url="<%= packageurl %>">
                 <%= javascript_tag do %>
-                    var ipackages = [ <%= @ipackages.map { |p| "#{p}" }.join(',').html_safe %>
-                    ]; renderPackagesProjectsTable({packages: ipackages, length: '25', name: 'ipackages_wrapper'});
+                    var ipackages = [ <%= escape_nested_list(@ipackages) %> ];
+                    renderPackagesProjectsTable({packages: ipackages, name: 'ipackages_wrapper'});
                 <% end %>
               </div>
           <% end %>

src/api/app/views/webui/project/list.html.erb

@@ -40,8 +40,8 @@
 
         <% content_for :head_javascript do %>
             <% cache ['list_public_arrays', cachekey] do %>
-                var main_projects = [ <%= escape_project_list(@main_projects) %> ];
-                var excl_projects = [ <%= escape_project_list(@excl_projects) %> ];
+                var main_projects = [ <%= escape_nested_list(@main_projects) %> ];
+                var excl_projects = [ <%= escape_nested_list(@excl_projects) %> ];
             <% end %>
         <% end %>
         <% content_for :ready_function do %>

src/api/app/views/webui/project/status.html.erb

@@ -145,7 +145,9 @@
           <% end %>
           <td>
             <span class="hidden"><%= sortkey %></span>
-            <%= raw outs.join("<br/>".html_safe) -%>
+            <% outs.each do |out| %>
+              <%= out %> <br />
+            <% end %>
           </td>
         </tr>
       <% end %>

src/api/app/views/webui/search/_owners.html.erb

@@ -10,6 +10,14 @@
     <p>
     <%= search_owners_list(result.users) %>
     <%= search_owners_list(result.groups, :group) %>
+    <% search_owners_list(result.groups, :group).each do |result| %>
+      <%= result %>
+      <br/>
+    <% end %>
+    <% search_owners_list(result.users).each do |result| %>
+      <%= result %>
+      <br/>
+     <% end %>
     </p>
   </div>
 <% end %>

src/api/app/views/webui/theme/bratwurst/layouts/webui/_navbar.html.haml

@@ -22,7 +22,7 @@
         %ul.nav.navbar-nav.navbar-right.hidden-xs
           %li.dropdown
             %a.dropdown-toggle{"data-toggle" => "dropdown", :href => "#", :id => "user-dropdown"}
-              = user_icon(User.current, 20)
+              = user_icon(User.current)
               %b.caret
             %ul.dropdown-menu
               %li

src/api/app/views/webui/user/edit.html.erb

@@ -19,11 +19,10 @@
     <p>
     <%= label_tag :globalrole, 'Admin:' %>
     <% if @displayed_user.is_admin? %>
-      <% checked = 'checked="checked"' %>
+      <input type="checkbox" name="globalrole" value="Admin" checked="checked" />
     <% else %>
-      <% checked = "" %>
+      <input type="checkbox" name="globalrole" value="Admin"/>
     <% end %>
-      <input type="checkbox" name="globalrole" value="Admin" <%= checked.html_safe if checked %> />
     </p>
     <p>
     <% @states.each do |s| %>