Project

General

Profile

Bug #25760

audit emails generate incorrect links

Added by Ohad Levy 5 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Audit Log
Target version:
Team Backlog:
Fixed in Releases:
Found in Releases:

Description

since the introducing of the new audit UI in 1.20, the audit summary emails generate a link with the audit ID in it, while in fact there is no more "show" action on rails controllers.

the emails should probably be changed to include a search query instead.


Related issues

Related to Foreman - Feature #24245: [RFE] New Audit UI as per new UX designClosed
Related to Foreman - Bug #25800: AuditList with one item should render it expandedClosed

Associated revisions

Revision 15ac3942 (diff)
Added by Gilad Lekner 5 months ago

Fixes #25760 - audit incorrect links

History

#1 Updated by Ohad Levy 5 months ago

  • Related to Feature #24245: [RFE] New Audit UI as per new UX design added

#2 Updated by Ohad Levy 5 months ago

I didn't test, but I think something like the following patch could fix the issue:

diff --git a/app/views/audit_mailer/summary.html.erb b/app/views/audit_mailer/summary.html.erb
index f1b490df9..fb6392840 100644
--- a/app/views/audit_mailer/summary.html.erb
+++ b/app/views/audit_mailer/summary.html.erb
@@ -11,10 +11,10 @@
           <%= link_to(audit.username, audits_url(:search => user_search_param)) %>
           <span style="color: #cccccc"><%= audit_remote_address audit %></span>
           <%= audit_action_name audit%> <%= audited_type audit %>:
-          <%= link_to(audit_title(audit), audit_url(:id => audit), {:style => 'color: #2a6496'}) %>
+          <%= link_to(audit_title(audit), audits_url(search: "id=#{audit.id}"), {:style => 'color: #2a6496'}) %>
         </b>
         <ul style="list-style-type: none;">
-          <% details(audit, audit_url(audit)).each do |detail| -%>
+          <% details(audit, audits_url(search: "id=#{audit.id}")).each do |detail| -%>
             <li>
               <%= detail %>
             </li>
diff --git a/app/views/audit_mailer/summary.text.erb b/app/views/audit_mailer/summary.text.erb
index f0862265f..8e341b76b 100644
--- a/app/views/audit_mailer/summary.text.erb
+++ b/app/views/audit_mailer/summary.text.erb
@@ -5,7 +5,7 @@
          'Displaying %{num_audits} of %{total_audits} audits', @count) % {:num_audits => @limit, :total_audits => @count} %>
   <% @audits.each do |audit| %>
     <%= audit.username %> (<%= audit.remote_address %>) <%= audit_action_name audit%> <%= audited_type audit %>: <%= audit_title(audit) %>
-    (<%= audit_url(:id => audit) %>)
+    (<%= audits_url(search: "id=#{audit}") %>)
   <% end %>
 <% else %>
   <%= _('No audit changes for this period') %>
diff --git a/config/routes.rb b/config/routes.rb
index b80a4edbf..75ace534e 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -264,7 +264,7 @@ Foreman::Application.routes.draw do
     end
   end

-  resources :audits do
+  resources :audits, :only => [:index] do
     collection do
       get 'auto_complete_search'
     end

it would be nice to expand the list if there is only one match to avoid the extra click

#3 Updated by Ohad Levy 5 months ago

  • Found in Releases 1.20.0 added

#4 Updated by Ohad Levy 5 months ago

for reference, the error that is shown on the UI when trying to follow the link:

Oops, we're sorry but something went wrong AuditsController#show is missing a template for this request format and variant. request.formats: ["text/html"] request.variant: [] NOTE! For XHR/Ajax or API requests, this action would normally respond with 204 No Content: an empty white screen. Since you're loading it in a web browser, we assume that you expected to actually render a template, not nothing, so we're showing an error to be extra-clear. If you expect 204 No Content, carry on. That's what you'll get from an XHR or API request. Give it a shot.

#5 Updated by The Foreman Bot 5 months ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/6376 added

#6 Updated by Tomer Brisker 5 months ago

  • Target version set to 1.20.2

#7 Updated by Tomer Brisker 5 months ago

  • Assignee set to Gilad Lekner

#8 Updated by Michael Moll 5 months ago

  • Fixed in Releases 1.21.0 added

#9 Updated by Anonymous 5 months ago

  • Status changed from Ready For Testing to Closed

#10 Updated by The Foreman Bot 5 months ago

  • Pull request https://github.com/theforeman/foreman/pull/6384 added

#11 Updated by Tomer Brisker 5 months ago

  • Fixed in Releases 1.20.2 added

#12 Updated by Ohad Levy 4 months ago

  • Related to Bug #25800: AuditList with one item should render it expanded added

Also available in: Atom PDF