Extract Actions

[koppen]

No description provided.

[Copilot]

🟡 Changes recommended

Unresolved action scoping, recordless rendering, execution, and dependency-loading issues block approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Extracts built-in CRUD actions into reusable, context-aware action objects rendered through dropdowns.

Changes:

• Adds configurable New, Edit, and Delete actions.
• Updates repository views and dropdown rendering.
• Expands action, repository, component, and controller tests.

File summaries

File	Summary
test/uchi/repository/actions_test.rb	Tests repository actions and scoping.
test/uchi/action_test.rb	Tests action configuration and rendering.
test/dummy/app/uchi/repositories/book.rb	Configures sample repository actions.
test/controllers/uchi/repository_controller_test.rb	Updates repository integration coverage.
test/components/uchi/ui/actions/dropdown_test.rb	Tests dropdown rendering.
lib/uchi/repository.rb	Adds default actions and context filtering; critical symbol-context mismatch remains.
lib/uchi/action/new.rb	Adds the New action; execution and translation issues remain.
lib/uchi/action/edit.rb	Adds the Edit action; recordless index rendering and translation issues remain.
lib/uchi/action/delete.rb	Adds the Delete action.
lib/uchi/action/configuration.rb	Adds action context configuration.
lib/uchi/action.rb	Adds shared rendering APIs; nil-record and direct-load issues remain.
lib/uchi.rb	Loads action classes.
app/views/uchi/repository/show.html.erb	Integrates scoped show actions.
app/views/uchi/repository/index.html.erb	Integrates index actions; empty header slot issue remains.
app/views/uchi/repository/edit.html.erb	Integrates scoped edit actions.
app/components/uchi/ui/actions/dropdown/dropdown.html.erb	Renders action controls; nil-record handling issue remains.
app/components/uchi/ui/actions/dropdown.rb	Configures dropdowns; unset time-zone handling issue remains.
app/assets/stylesheets/uchi/application.css	Adds dropdown positioning styles.

Review details

Suppressed comments (8)

app/components/uchi/ui/actions/dropdown.rb:37

• For the recordless dropdown used by the index view (record: nil), this calls Time.zone.now; Rails applications may not configure Time.zone (the dummy app leaves it unset), so rendering a multi-action index raises NoMethodError. Use a zone-independent Rails clock here.

          @record_id ||= record&.id || Time.zone.now.to_i

app/components/uchi/ui/actions/dropdown/dropdown.html.erb:2

• The index page passes record: nil, so a single generic action reaches button_render with no record; the base implementation immediately calls record.id and raises. This also affects any record-dependent action configured for index, not just the built-in Edit action. Make the single-action path support nil/header actions or require and supply a record before calling button_render.

  <%= actions.first.button_render(record: record, repository: repository, view: self.view_context) %>

app/views/uchi/repository/index.html.erb:14

• Unlike the show and edit templates, this unconditionally registers a page-header action slot even when actions_for(:index) is empty. PageHeader checks whether slots exist, not whether their rendered content is nonempty, so repositories with no index actions get an empty action container in the header.

  <% header.with_action do %>
    <%= render(Uchi::Ui::Actions::Dropdown.new(
      actions: @repository.actions_for(Uchi::View::INDEX),
      record: nil,
      repository: @repository
    )) %>
  <% end %>

lib/uchi/action.rb:123

• The index view passes record: nil, and the base action's default on includes :index. A single custom action that inherits this method therefore calls nil.id before it can render. Make the id field conditional and compact the fields as #render already does.

          view.hidden_field_tag(:id, record.id),

lib/uchi/action.rb:3

• Configuration#initialize now calls default_on, which references Uchi::View, but this file does not load the view definitions. A direct require "uchi/action" followed by Uchi::Action.new therefore raises NameError; load the view dependency here as the field configuration does.

require_relative "action/configuration"

lib/uchi/action/edit.rb:30

• Like New#perform, this method is reachable through the generic action execution endpoint, but repository is not defined on Action. Invoking the registered default Edit action directly therefore raises NoMethodError and returns a 500 response.

        Uchi::ActionResponse.success.redirect_to(
          path: repository.routes.path_for(:edit, id: record.id)
        )

lib/uchi/action/edit.rb:37

• This standalone button also uses Action#name rather than repository.translate.link_to_edit(record), so localized Edit labels are ignored when Edit is the only action. Use the repository translation here as well.

          name,

lib/uchi/action/new.rb:32

• This standalone button also uses Action#name rather than the repository-specific repository.translate.link_to_new lookup, so localized link_to_new values are ignored when New is the only action. Use the repository translation here as well.

          name,

• Files reviewed: 18/18 changed files
• Comments generated: 7
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Unresolved action rendering and built-in action execution defects block approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

lib/uchi/action.rb:123

• The index template passes record: nil, and a single collection-scoped custom action uses this default button_render; unlike render, this line unconditionally calls record.id, so an action that does not require a specific record still crashes when it is the only index action. Make the id field conditional (and compact the fields) as the regular renderer already does.

          view.hidden_field_tag(:id, record.id),

lib/uchi/action/edit.rb:13

• Using the generic action name here drops the repository-specific label that the previous edit link used. For example, da.yml defines uchi.repository.author.button.link_to_edit, but this path now falls back to the generic "Edit"; use repository.translate.link_to_edit(record) instead.

          name,

lib/uchi/action/edit.rb:37

• Using the generic action name here drops the repository-specific label that the previous edit link used. For example, da.yml defines uchi.repository.author.button.link_to_edit, but this path now falls back to the generic "Edit"; use repository.translate.link_to_edit(record) instead.

          name,

lib/uchi/action/new.rb:15

• perform is part of the action contract and the executions controller will call it for any posted action_name, but repository is not a local or method on Uchi::Action (only the rendering methods receive it). Invoking the built-in New action through the execution endpoint therefore raises NameError instead of redirecting; either make the action non-executable or provide it with a repository before calling perform.

        Uchi::ActionResponse.success.redirect_to(
          path: repository.routes.path_for(:new)
        )

lib/uchi/action/new.rb:22

• Using the generic action name here drops the repository-specific label that the previous new link used. For example, da.yml defines uchi.repository.author.button.link_to_new, but this path now displays the generic "New"; use repository.translate.link_to_new instead.

          name,

lib/uchi/action/new.rb:32

• Using the generic action name here drops the repository-specific label that the previous new link used. For example, da.yml defines uchi.repository.author.button.link_to_new, but this path now displays the generic "New"; use repository.translate.link_to_new instead.

          name,

• Files reviewed: 18/18 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

New and Edit actions must preserve repository-specific localized labels.

Review details

Suppressed comments (4)

lib/uchi/action/edit.rb:13

• This replaces the previous repository-localized edit label with the generic action name. repository.translate.link_to_edit(record) is the established API for this text and supports per-model translations (for example, “Rediger bog”); use it here instead of name.

          name,

lib/uchi/action/edit.rb:37

• The edit link in the dropdown likewise bypasses repository.translate.link_to_edit(record), so localized and repository-specific labels are lost. Use that translation helper here rather than the generic action name.

          name,

lib/uchi/action/new.rb:22

• This uses the generic action name instead of the repository-localized label. The existing translation contract is repository.translate.link_to_new (see lib/uchi/repository/translate.rb:215), so locales such as the dummy Danish locale lose labels like “Ny forfatter” and fall back to “New”. Use the repository translation here.

          name,

lib/uchi/action/new.rb:32

• The standalone New button also bypasses the repository-specific button.link_to_new translation and displays the generic action name. Use repository.translate.link_to_new so this extracted action preserves the localization behavior of the previous index header link.

          name,

• Files reviewed: 20/20 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The unresolved critical Delete regression and moderate Edit label regression block approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

test/components/uchi/ui/actions/dropdown_test.rb:32

• The test description has a subject/verb agreement error: “the number of actions don't exceed” should use “doesn't exceed.”

        test "renders both actions directly, without a dropdown menu, when the number of actions don't exceed the configured number" do

• Files reviewed: 21/21 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved moderate action-rendering, context, identifier, and API visibility issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

app/components/uchi/ui/actions/dropdown.rb:51

• Using Time.current.to_i as the identifier for a collection-level dropdown makes every such component rendered in the same second share the same button/menu IDs. If a page renders more than one index action component, Stimulus and aria-labelledby can target the wrong menu; use a per-component unique value (for example the component's object_id) for the nil-record case.

          @record_id ||= record&.id || Time.current.to_i

lib/uchi/action/new.rb:50

• Because this method is defined after protected, Uchi::Action::New.new.name is now protected even though Action#name is a public API and the actions documentation tells consumers to override/call #name. This also breaks callers that inspect or label the built-in New action directly. Keep name public by moving it above protected or adding public :name.

      def name
        return super unless repository

        repository.translate.link_to_new

• Files reviewed: 23/23 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

lib/uchi/action/configuration.rb can raise NameError because it does not load Uchi::View directly.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 23/23 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

One critical and two moderate unresolved issues remain in action context handling, action visibility, and repository title behavior.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

lib/uchi/action/new.rb:50

• Action#name is public, but the protected declaration above this method makes Uchi::Action::New#name protected. This breaks the base action API for callers that inspect or customize the built-in action (Uchi::Action::New.new.name raises NoMethodError); expose this override publicly like the inherited method.

      def name
        return super unless repository

        repository.translate.link_to_new

test/dummy/app/uchi/repositories/book.rb:23

• Repository#title explicitly returns nil for a nil record, but this override now returns the Book repository class instead. Any caller that asks for a title without a record will receive a Class object, which is not a display title and can leak into translation/interpolation code. Preserve the base contract by returning nil when model is absent.

        return self.class unless model

• Files reviewed: 23/23 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Delete actions must preserve feedback messages, and the documented Delete-on-index example must be corrected.

Review details

Suppressed comments (2)

docs/actions.md:44

• This example adds Delete to :index, but the index header passes record: nil and Delete renders record.id/record-specific translations, so copying this documented configuration raises while rendering the index. Since record-bound index actions are not supported, use a record-independent action here or scope Delete only to a record view.

        super + [Uchi::Action::Delete.new.on(:index, :edit)]

lib/uchi/action/delete.rb:15

• This drops the localized feedback that the existing RepositoryController#destroy supplies: both successful deletes and callback failures now redirect without any flash message, so a failed delete appears to succeed silently. Return repository.translate.successful_destroy / repository.translate.failed_destroy in these responses (and preserve the failure redirect if that behavior is required).

        if destroyed.all?
          Uchi::ActionResponse.success
        else
          Uchi::ActionResponse.error

• Files reviewed: 23/23 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Repository-level index actions currently receive an empty record set and require corrected execution behavior.

Review details

Suppressed comments (1)

lib/uchi/action.rb:125

• When an action is rendered on :index, record is nil and this intentionally omits the id field, but ExecutionsController#find_records turns a missing id/ids into [] and calls repository.find_many([]). Consequently, repository-level index actions receive an empty relation and are no-ops, contrary to the documentation that index actions apply to the repository as a whole. Define how index executions load their records (for example, pass the repository's full scope) before omitting the record ID.

          (record ? view.hidden_field_tag(:id, record.id) : nil),

• Files reviewed: 23/23 changed files
• Comments generated: 0 new
• Review effort level: Lite