Skip to content

feat: added controller, views, tests for reports#240

Closed
AliK070 wants to merge 8 commits intomainfrom
report-index-page
Closed

feat: added controller, views, tests for reports#240
AliK070 wants to merge 8 commits intomainfrom
report-index-page

Conversation

@AliK070
Copy link
Contributor

@AliK070 AliK070 commented Feb 24, 2026

TL;DR

Adds controller routes, views, and tests for displaying all reports using the IndexTableComponent. Also has edit and delete functionality, and ensures the page matches the style and systems of other index pages.

What is this PR trying to achieve?

-closes #113

How did you achieve it?

  • Added routes for reports (index, show, edit new, create, update, destroy), in routes.rb.
  • Created index using the IndexTableComponent.
  • Updated and normalized locale files for both English and Spanish

Checklist

  • Changes have been top-hatted locally
  • Tests have been added or updated
  • Documentation has been updated (if applicable)
  • Linked related issues

@AliK070 AliK070 requested a review from pranavrao145 February 24, 2026 17:29
Comment on lines +25 to +26
SIDEBAR_ITEM.new(name: t("reports", scope: "components.sidebar_component.build_admin_section"),
path: helpers.reports_path, icon: "reports-fill")
Copy link
Contributor

Choose a reason for hiding this comment

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

Make this a seperate pr.

Copilot AI review requested due to automatic review settings March 14, 2026 02:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a Reports index page and navigation entry so admins can view reports in a standardized index-table layout, aligning the Reports area with other admin index pages.

Changes:

  • Adds reports#index action plus a new app/views/reports/index.html.erb using Shared::IndexTableComponent.
  • Adds a “Reports” link to the admin sidebar and introduces a new sidebar icon.
  • Adds EN/ES i18n keys for the new sidebar label under the reports locale files.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
config/locales/es/reports.es.yml Adds Spanish i18n key for the new sidebar “Reports” label (currently placed under components.sidebar_component...).
config/locales/en/reports.en.yml Adds English i18n key for the new sidebar “Reports” label (currently placed under components.sidebar_component...).
app/views/reports/index.html.erb New Reports index page using Shared::IndexTableComponent with view/edit/delete action buttons.
app/controllers/reports_controller.rb Adds index action and a before_action for report loading.
app/components/sidebar_component.rb Adds a Reports item to the admin sidebar section.
app/assets/images/icons/reports-fill.svg Adds a new SVG icon for the sidebar.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +10 to +16
<% 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| %>
Comment on lines +2 to +6
before_action :set_report, only: %i[show edit update destroy]

def index
@reports = Report.all
end
Comment on lines +3 to +16
<h1 class="text-4xl font-bold"><%= t(".title") %></h1>
<%= render ActionButtonComponent.new(to: new_report_path, icon: "add", colour: :primary, size: :large, turbo_stream: true) 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 %>
<% 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| %>
Comment on lines +21 to +22
<%= render ActionButtonComponent.new(to: report_path(report), icon: "view", turbo_stream: true) %>
<%= render ActionButtonComponent.new(to: edit_report_path(report), icon: "edit", turbo_stream: true, colour: :info) %>
<div class="flex flex-row gap-2 items-center">
<%= render ActionButtonComponent.new(to: report_path(report), icon: "view", turbo_stream: true) %>
<%= render ActionButtonComponent.new(to: edit_report_path(report), icon: "edit", turbo_stream: true, colour: :info) %>
<%= render ActionButtonComponent.new(to: report_path(report), method: :delete, icon: "delete", colour: :error, confirm: t("reports.destroy.confirm")) %>
<div class="flex flex-row gap-2 items-center">
<%= render ActionButtonComponent.new(to: report_path(report), icon: "view", turbo_stream: true) %>
<%= render ActionButtonComponent.new(to: edit_report_path(report), icon: "edit", turbo_stream: true, colour: :info) %>
<%= render ActionButtonComponent.new(to: report_path(report), method: :delete, icon: "delete", colour: :error, confirm: t("reports.destroy.confirm")) %>
path: helpers.regions_path, icon: "compass")
path: helpers.regions_path, icon: "compass"),
SIDEBAR_ITEM.new(name: t("reports", scope: "components.sidebar_component.build_admin_section"),
path: helpers.reports_path, icon: "reports-fill")
Comment on lines +3 to +6
components:
sidebar_component:
build_admin_section:
reports: Reports
Comment on lines +3 to +6
components:
sidebar_component:
build_admin_section:
reports: Informes
<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, turbo_stream: true) do %>
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.

Comment on lines +64 to +86
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.

Comment on lines +10 to +12
<% 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.

Copilot AI review requested due to automatic review settings March 20, 2026 03:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a full Reports index page (using Shared::IndexTableComponent) plus routing/controller support and tests, aligning the Reports area with existing index-style pages and enabling deletion.

Changes:

  • Add reports#index route/controller action and a new reports/index.html.erb table view with view/edit/delete actions.
  • Add reports#destroy route/controller action and corresponding flash translations (EN/ES).
  • Update controller tests to cover index display and destroy behavior; refactor tests away from the removed filter route.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
app/controllers/reports_controller.rb Adds index to list reports and destroy to delete a report.
app/views/reports/index.html.erb New index page rendering reports via Shared::IndexTableComponent with action buttons.
config/routes.rb Replaces the old filter collection route with full index + destroy REST routes.
test/controllers/reports_controller_test.rb Adds index and destroy coverage; updates tests to use new instead of filter.
config/locales/en/reports.en.yml Adds index/destroy strings for the new UI/actions.
config/locales/es/reports.es.yml Adds index/destroy strings for the new UI/actions.
app/assets/images/icons/reports-fill.svg Adds a new “reports” icon asset.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

get :filter
end
end
resources :reports, only: %i[index show new create edit update destroy]
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.

By removing the filter collection route, filter_reports_path/filter_reports_url no longer exists. The existing app/views/reports/new.html.erb still submits its GET form to filter_reports_path, so the New Report page will raise a routing error. Update that form to submit to new_report_path (and rely on ReportsController#new for filtering), or reintroduce a dedicated filter route/action.

Suggested change
resources :reports, only: %i[index show new create edit update destroy]
resources :reports, only: %i[index show new create edit update destroy] do
collection do
get :filter
end
end

Copilot uses AI. Check for mistakes.
Comment on lines +10 to +16
<% 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| %>
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.
index:
actions:
create: Create Report
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.
title: Editar Reporte
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.
Comment on lines +194 to +202
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

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.
@AliK070 AliK070 requested a review from CulmoneY March 20, 2026 03:39
@AliK070 AliK070 closed this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Report Index Page

3 participants