Fix review issues #62-#66 filed against the Avo installer (PR #52) - #83
Open
kanejamison wants to merge 7 commits into
Open
Fix review issues #62-#66 filed against the Avo installer (PR #52)#83kanejamison wants to merge 7 commits into
kanejamison wants to merge 7 commits into
Conversation
ENV["EDITOR"] collides with the universal shell $EDITOR variable, so EDITOR=vim silently fell through to the textarea branch. The installer now reads BUNKO_EDITOR instead, validates the value against markdown/rhino/tiptap/trix/textarea, and aborts with a message listing the valid options on anything else. Task help output, README docs, and tests updated accordingly. Closes #62 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAFZmU5SJSERBVyedpsYQW
The generated filter called query.by_post_type(post_type) with a PostType record, but Post.by_post_type expects a name string, so the filter silently matched nothing. The template now filters on the association directly with query.where(post_type: post_type), and the task test asserts the corrected query is generated. Closes #64 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAFZmU5SJSERBVyedpsYQW
The generated actions called post.update in a loop and unconditionally reported success, silently swallowing validation failures. They now check each update's return value, collect failures with record identifiers and error messages, and report through Avo's error/warn APIs: plain succeed only when everything succeeded, error when every record failed, and warn with succeeded/failed counts for mixed batches. Closes #66 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAFZmU5SJSERBVyedpsYQW
The generated Post resource's search block uses Ransack, but Ransack 4+ raises unless the model defines ransackable_attributes, and ransack may not even be in the host bundle. The installer now prints a clearly worded post-install step with the explicit allowlist to add to app/models/post.rb: def self.ransackable_attributes(auth_object = nil) = %w[id title slug] Only id/title/slug are allowlisted — never status or content, since Avo search also runs over drafts. The generated resource template and the README document the same allowlist plus the behavior when ransack is absent (remove the search block, or add the gem). Closes #63 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAFZmU5SJSERBVyedpsYQW
The bunko:avo:install "Next steps" output only said to visit http://localhost:3000/avo, but the generated admin is wide open until Avo auth is configured. The output now includes an explicit numbered security step telling users to configure Avo's authenticate_with (and authorization) before deploying, linking to Avo's authentication docs. The README's Avo section mirrors the same warning. Closes #65 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAFZmU5SJSERBVyedpsYQW
test_sample_data_adds_root_route_for_home_page failed whenever an earlier test in the run had loaded the dummy app's routes, which define root "blog#index": the task's root_route_exists? check saw the live :root named route and skipped generating the home page. The test now redraws the live route set without a root route before invoking the task and restores the dummy routes afterwards, so it passes regardless of test order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAFZmU5SJSERBVyedpsYQW
The pages_controller.rb.tt template used a single-quoted path separator, so the regenerated dummy-app controller failed Standard lint and broke `bundle exec rake` on this branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAFZmU5SJSERBVyedpsYQW
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes the five review issues filed against PR #52 (the
bunko:avo:installAvo admin installer). It targets theset-up-an-avo-installer-rakebranch so the fixes flow into that PR.EDITORenv var collision: The installer now readsBUNKO_EDITORinstead ofENV["EDITOR"](which collided with the universal shell$EDITOR, soEDITOR=vimsilently fell through to the textarea branch). The value is validated againstmarkdown/rhino/tiptap/trix/textareaand the task aborts with a message listing valid options on anything else. Task help output, README, and tests updated; new tests cover the invalid-value abort and that the shell$EDITORis ignored.app/models/post.rb:def self.ransackable_attributes(auth_object = nil) = %w[id title slug]— onlyid/title/slug, neverstatusorcontent, since Avo search also runs over drafts. The generated resource template and the README document the same allowlist plus the behavior when ransack is absent from the bundle (add the gem or remove the search block).PostTyperecord toPost.by_post_type, which expects a name string, so it silently matched nothing. It now usesquery.where(post_type: post_type); the task test asserts the corrected query is generated.authenticate_with/authorization is configured, linking to https://docs.avohq.io/3.0/authentication.html. The README's Avo section mirrors the warning.PublishPost/UnpublishPostactions now check eachupdate's return value, collect failures with record identifiers and error messages, and report through Avo'serror/warnAPIs — plainsucceedonly when everything succeeded,errorwhen every record failed, andwarnwith succeeded/failed counts for mixed batches.Two small housekeeping commits were also needed to make
bundle exec rakegreen on this branch: a fix for an order-dependent assertion in the sample-data root-route test (the live dummy-app root route maderoot_route_exists?true depending on test order), and a Standard lint fix in the generatedpages_controller.rb.tttemplate (single-quoted string).Closes #62
Closes #63
Closes #64
Closes #65
Closes #66
Test results
bundle exec rake(tests + standardrb): 374 runs, 1406 assertions, 0 failures, 0 errors, 1 skip (the skip is pre-existing); Standard lint clean.🤖 Generated with Claude Code
Generated by Claude Code