Added reports navigation Item in sidebar#265
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a Reports entry to the admin sidebar and introduces an index page for listing/deleting reports, with supporting routes, translations, icons, and controller/tests.
Changes:
- Added
reports#indexUI (table + actions) and wired upreports#index/reports#destroyroutes and controller actions. - Added sidebar navigation item for Reports, plus EN/ES i18n keys for sidebar + index/destroy copy.
- Added controller test coverage for index/destroy behavior and authorization redirects.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
test/controllers/reports_controller_test.rb |
Extends auth coverage for index/destroy and adds basic index + destroy behavior tests. |
config/routes.rb |
Exposes reports#index and reports#destroy. |
config/locales/es/reports.es.yml |
Adds ES strings for index + destroy + Report attribute labels. |
config/locales/es/components.es.yml |
Adds ES sidebar label for “Reports”. |
config/locales/en/reports.en.yml |
Adds EN strings for index + destroy + Report attribute labels. |
config/locales/en/components.en.yml |
Adds EN sidebar label for “Reports”. |
app/views/reports/index.html.erb |
New index page rendering reports table and action buttons. |
app/controllers/reports_controller.rb |
Adds index and destroy actions. |
app/components/sidebar_component.rb |
Adds Reports item in admin sidebar section. |
app/assets/images/icons/reports-fill.svg |
New icon asset used by sidebar item. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <%= render ActionButtonComponent.new(to: new_report_path, icon: "add", colour: :primary, size: :large, turbo_stream: true) do %> | ||
| <%= t(".actions.create") %> | ||
| <% end %> |
There was a problem hiding this comment.
ActionButtonComponent is rendering this link with data-turbo-stream, but ReportsController#new doesn’t declare a turbo_stream response and there’s no app/views/reports/new.turbo_stream.erb. This will cause a MissingTemplate (or an unexpected turbo_stream format) when the button is clicked. Either remove turbo_stream: true here, or add respond_to + a new.turbo_stream.erb template (as done in Projects/Regions/Users).
| <% table.column :actions, header: t("shared.index_table_component.actions"), col_size: "90px" do |report| %> | ||
| <div class="flex items-center gap-2"> | ||
| <%= render ActionButtonComponent.new(to: report_path(report), icon: "view") %> | ||
| <%= render ActionButtonComponent.new(to: edit_report_path(report), icon: "edit", colour: :info, turbo_stream: true) %> |
There was a problem hiding this comment.
This edit action button is sent as a Turbo Stream request (turbo_stream: true), but ReportsController#edit currently doesn’t respond_to :turbo_stream and there’s no edit.turbo_stream.erb template under app/views/reports/. This can raise MissingTemplate when navigating to edit from the index.
| <%= render ActionButtonComponent.new(to: edit_report_path(report), icon: "edit", colour: :info, turbo_stream: true) %> | |
| <%= render ActionButtonComponent.new(to: edit_report_path(report), icon: "edit", colour: :info) %> |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Report routes | ||
| resources :reports do | ||
| resources :reports, only: %i[index show new create edit update destroy] do |
There was a problem hiding this comment.
resources :reports already generates all 7 RESTful routes by default, so listing only: %i[index show new create edit update destroy] is redundant and adds upkeep if routes change later. Either remove the only: entirely or restrict it to the minimal set of actions you actually intend to expose.
| resources :reports, only: %i[index show new create edit update destroy] do | |
| resources :reports do |
|
|
||
| # Report routes | ||
| resources :reports do | ||
| resources :reports, only: %i[index show new create edit update destroy] do |
There was a problem hiding this comment.
PR description checklist indicates tests were added/updated, but this PR doesn't include any test changes in the diff. If this is intentional, consider updating the checklist; otherwise, add/adjust tests to reflect the UI/i18n changes.
| columns: | ||
| end_date: End date | ||
| id: ID | ||
| start_date: Start date |
There was a problem hiding this comment.
These reports.index.columns.* translations duplicate the already-present activerecord.attributes.report.* entries in the same file. If the table can rely on human_attribute_name (or you reference the activerecord keys), you can remove this duplicated block to keep i18n DRY.
| columns: | |
| end_date: End date | |
| id: ID | |
| start_date: Start date |
| columns: | ||
| end_date: Fecha de fin | ||
| id: ID | ||
| start_date: Fecha de inicio |
There was a problem hiding this comment.
These reports.index.columns.* translations duplicate the already-present activerecord.attributes.report.* entries in the same file. If the table can rely on human_attribute_name (or you reference the activerecord keys), you can remove this duplicated block to keep i18n DRY.
| columns: | |
| end_date: Fecha de fin | |
| id: ID | |
| start_date: Fecha de inicio |
TL;DR
Added icon and navigation for the reports view page.
What is this PR trying to achieve?
To add navigation for viewing the reports page.
How did you achieve it?
Added the Reports sidebar navigation item (using the reports-fill icon) and adds the i18n keys so the label appears correctly in supported locales.
Checklist