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]

Copilot review overview

🔵 Needs a closer look

The Book repository returns a class instead of nil when asked for a missing record title, violating the repository title contract.

Review effort: Balanced
Findings: 2 · 1

Open (3)

• Default rake task fails without installing Node dependencies
• Handle action subclasses that skip Configuration initialization
• Preserve nil result in title override

Resolved since last review (1)

• Redirect successful deletion to the repository index

[Copilot]

Copilot review overview

🟡 Changes recommended

The Book repository violates the nil-title contract, and several new APIs need documentation and ordering corrections.

Review effort: Balanced
Findings: 1 · 1 · 4

Open (6)

• Default rake task fails without installing Node dependencies
• Preserve nil result in title override
• Correct getter return type in public API documentation · New
• Alphabetize public rendering methods · New
• Alphabetize public rendering methods · New
• Alphabetize public rendering methods · New

Resolved since last review (1)

• Handle action subclasses that skip Configuration initialization

[Copilot]

Copilot review overview

🔵 Needs a closer look

Dropdown-rendered New and Edit links do not preserve their standalone Turbo navigation behavior.

Review effort: Balanced
Findings: 1

Open (1)

• Default rake task fails without installing Node dependencies

Resolved since last review (5)

• Preserve nil result in title override
• Alphabetize public rendering methods
• Alphabetize public rendering methods
• Alphabetize public rendering methods
• Correct getter return type in public API documentation

Previously missed (2)

In code that hasn't changed since last review

Dropdown Edit link omits the _top Turbo target

lib/​uchi/​action/​edit.rb:40

When Edit is pushed into the dropdown, this link omits the _top Turbo target that render_as_button sets. Consequently, merely changing action ordering or the dropdown threshold changes whether Edit performs full-page navigation or targets an enclosing Turbo Frame. Keep both render modes behaviorally equivalent.

Dropdown New link omits the _top Turbo target

lib/​uchi/​action/​new.rb:46

When New is pushed into the dropdown (for example, by lowering the repository threshold or placing earlier actions first), this link loses the _top Turbo target used by render_as_button. If the repository page is itself rendered in a Turbo Frame, the same action then navigates only that frame depending on its dropdown placement. Preserve the target in both render paths.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Custom actions can fail to render because the supplied view context does not expose uchi_path_to.

Review effort: Balanced
Findings: None

Resolved since last review (1)

• Default rake task fails without installing Node dependencies

Previously missed (1)

In code that hasn't changed since last review

Custom action rendering raises NoMethodError without route helpers

lib/​uchi/​action.rb:83

view is the underlying ActionView context, but uchi_path_to is only mixed into the dropdown component. Applications with config.action_controller.include_all_helpers = false (including the dummy app at test/dummy/config/application.rb:14) therefore raise NoMethodError when a normal custom action is rendered outside the dropdown. Build the execution URL through an API available to the view context, or pass a rendering object that includes Uchi::RoutesHelper.

This issue also appears on line 106 of the same file.