Skip to content
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/assets/images/icons/reports-fill.svg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This icon is not used throughout this PR.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
62 changes: 38 additions & 24 deletions app/controllers/reports_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
class ReportsController < ApplicationController
def index
@reports = Report.all
end

def show
@report = Report.find(params[:id])
end
Expand All @@ -11,30 +15,6 @@ def edit
render :show
end

def filter
start_date = parse_date(params[:start_date])
end_date = parse_date(params[:end_date])

if start_date && end_date && start_date <= end_date
@start_date = params[:start_date]
@end_date = params[:end_date]

result = ReportFilterService.new.filter(
start_date: start_date,
end_date: end_date,
project_ids: Array(params[:project_ids]),
subproject_ids: Array(params[:subproject_ids])
)

@projects = result.projects
@selected_project_ids = result.selected_project_ids
@subprojects = result.subprojects
@selected_subproject_ids = result.selected_subproject_ids
end

render :new
end

def create
start_date = parse_date(params[:start_date])
end_date = parse_date(params[:end_date])
Expand Down Expand Up @@ -71,6 +51,40 @@ def create
redirect_to edit_report_path(report), flash: { success: t(".success") }
end

def destroy
@report = Report.find(params[:id])

if @report.destroy
redirect_to reports_path, flash: { success: t(".success") }
else
redirect_to reports_path, flash: { error: @report.errors.full_messages.to_sentence }
end
end

def filter
start_date = parse_date(params[:start_date])
end_date = parse_date(params[:end_date])

if start_date && end_date && start_date <= end_date
@start_date = params[:start_date]
@end_date = params[:end_date]

result = ReportFilterService.new.filter(
start_date: start_date,
end_date: end_date,
project_ids: Array(params[:project_ids]),
subproject_ids: Array(params[:subproject_ids])
)

@projects = result.projects
@selected_project_ids = result.selected_project_ids
@subprojects = result.subprojects
@selected_subproject_ids = result.selected_subproject_ids
end

render :new
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be in the diff.


private

def parse_date(value)
Expand Down
28 changes: 28 additions & 0 deletions app/views/reports/index.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<div class="mx-8 my-10">
<div class="flex items-center justify-between mb-4">
<h1 class="text-4xl font-bold"><%= t(".title") %></h1>
<%= render ActionButtonComponent.new(to: new_report_path, icon: "add", colour: :primary, size: :large) do %>
<%= t(".actions.create") %>
<% end %>
</div>
<%= render ContentCardComponent.new do %>
<%= render Shared::IndexTableComponent.new(records: @reports) do |table| %>
<% table.column :start_date, header: t(".columns.start_date") do |report| %>
<%= l(report.start_date) if report.start_date %>
<% end %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need the header here. The table component derives it.

<% table.column :end_date, header: t(".columns.end_date") do |report| %>
<%= l(report.end_date) if report.end_date %>
<% end %>
<% table.column :created_at, header: t(".columns.created_at") do |report| %>
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The table headers use t(".columns.start_date") / end_date / created_at, but there are no corresponding reports.index.columns.* keys in the locale files (they were not added in this PR). This will display "translation missing" in the UI. Either add those locale keys or omit the explicit header: and let IndexTableComponent fall back to Report.human_attribute_name.

Suggested change
<% table.column :start_date, header: t(".columns.start_date") do |report| %>
<%= l(report.start_date) if report.start_date %>
<% end %>
<% table.column :end_date, header: t(".columns.end_date") do |report| %>
<%= l(report.end_date) if report.end_date %>
<% end %>
<% table.column :created_at, header: t(".columns.created_at") do |report| %>
<% table.column :start_date do |report| %>
<%= l(report.start_date) if report.start_date %>
<% end %>
<% table.column :end_date do |report| %>
<%= l(report.end_date) if report.end_date %>
<% end %>
<% table.column :created_at do |report| %>

Copilot uses AI. Check for mistakes.
<%= l(report.created_at) %>
<% end %>
<% table.column :actions, header: t("shared.index_table_component.actions"), col_size: "90px" do |report| %>
<div class="flex flex-row gap-2 items-center">
<%= render ActionButtonComponent.new(to: report_path(report), icon: "view") %>
<%= render ActionButtonComponent.new(to: edit_report_path(report), icon: "edit", colour: :info) %>
<%= render ActionButtonComponent.new(to: report_path(report), method: :delete, icon: "delete", colour: :error, confirm: t("reports.destroy.confirm")) %>
</div>
<% end %>
<% end %>
<% end %>
</div>
11 changes: 11 additions & 0 deletions config/locales/en/reports.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ en:
create:
invalid: Please provide valid dates and select at least one subproject.
success: Report was successfully generated.
destroy:
confirm: Are you sure you want to delete this report?
success: Report deleted successfully.
index:
actions:
create: Create Report
columns:
created_at: Created At
end_date: End Date
start_date: Start Date
title: Reports
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reports.index translations were added, but the view expects reports.index.columns.start_date/end_date/created_at. Add a columns: block under reports.index (and keep keys consistent with the view), otherwise the index page will show missing-translation placeholders for the column headers.

Suggested change
title: Reports
title: Reports
columns:
start_date: Start Date
end_date: End Date
created_at: Created At

Copilot uses AI. Check for mistakes.
journal_card_component:
author: Author
new:
Expand Down
11 changes: 11 additions & 0 deletions config/locales/es/reports.es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ es:
create:
invalid: Por favor proporcione fechas válidas y seleccione al menos un subproyecto.
success: El reporte fue generado exitosamente.
destroy:
confirm: "¿Está seguro de que desea eliminar este informe?"
success: Informe eliminado exitosamente.
index:
actions:
create: Crear informe
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reports.index translations were added, but the index view expects reports.index.columns.start_date/end_date/created_at. Add a columns: block under reports.index with these keys to avoid "translation missing" headers.

Suggested change
create: Crear informe
create: Crear informe
columns:
start_date: Fecha de inicio
end_date: Fecha de fin
created_at: Fecha de creación

Copilot uses AI. Check for mistakes.
columns:
created_at: Creado en
end_date: Fecha de finalizacion
start_date: Fecha de inicio
title: Informes
journal_card_component:
author: Autor
new:
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
delete "/logout", to: "sessions#destroy"

# Report routes
resources :reports, only: %i[show new create edit] do
resources :reports, only: %i[index show new create edit destroy] do
collection do
get :filter
end
Expand Down
52 changes: 52 additions & 0 deletions test/controllers/reports_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest
end

[
{ route: "index", method: :get, url_helper: :reports_url, needs_report: false },
{ route: "show", method: :get, url_helper: :report_url, needs_report: true },
{ route: "new", method: :get, url_helper: :new_report_url, needs_report: false },
{ route: "edit", method: :get, url_helper: :edit_report_url, needs_report: true },
Expand Down Expand Up @@ -57,6 +58,15 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest
assert_match aggregated_datum.additional_text, response.body
end

test "#index displays reports" do
report = create(:report)

get reports_path
assert_response :success
assert_match I18n.l(report.start_date.to_date), response.body
assert_match I18n.l(report.end_date.to_date), response.body
end

test "#filter displays projects when valid dates are provided" do
subproject = create(:subproject)
create(:log_entry, subproject: subproject, created_at: 1.day.ago)
Expand Down Expand Up @@ -160,4 +170,46 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to new_report_path
assert_equal I18n.t("reports.create.invalid"), flash[:error]
end

test "#create redirects with error when subproject_ids do not exist" do
assert_no_difference("Report.count") do
post reports_path, params: {
start_date: Time.zone.yesterday.to_s,
end_date: Time.zone.today.to_s,
subproject_ids: [0]
}
end

assert_redirected_to new_report_path
assert_equal I18n.t("reports.create.invalid"), flash[:error]
end

test "#destroy redirects to login route when a user is not authenticated" do
report = create(:report)
log_out_user

delete report_path(report)
assert_response :redirect
assert_redirected_to login_path
end

Comment on lines +193 to +201
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#destroy already has unauthenticated/unauthorized coverage via the parameterized route tests at the top of this file. The dedicated #destroy redirects to login route when a user is not authenticated test below is redundant and increases maintenance cost; consider removing it or removing destroy from the parameterized list to keep a single source of truth.

Suggested change
test "#destroy redirects to login route when a user is not authenticated" do
report = create(:report)
log_out_user
delete report_path(report)
assert_response :redirect
assert_redirected_to login_path
end

Copilot uses AI. Check for mistakes.
test "#destroy redirects to root route when a user is not authorized" do
report = create(:report)
create_logged_in_user

delete report_path(report)
assert_response :redirect
assert_redirected_to root_path
end

test "#destroy removes the report and redirects to index" do
report = create(:report)

assert_difference("Report.count", -1) do
delete report_path(report)
end

assert_redirected_to reports_path
assert_equal I18n.t("reports.destroy.success"), flash[:success]
end
end
Loading