feat: surface unaccepted contributor payouts as an admin task - #181
Open
billie-clawford-stacksbot[bot] wants to merge 7 commits into
Open
billie-clawford-stacksbot[bot] wants to merge 7 commits into
billie-clawford-stacksbot[bot] wants to merge 7 commits into
Conversation
… CI failure) sync_all's sync_all test didn't stub Stacks::Etl::Groups::Connector when stacks:etl:sync_google_groups was wired into sync_all (50bd261). CI's own test run for stacks:etl:sync_all was therefore invoking the REAL connector, which fetched real Google Group threads and tried to index chunks — creating an Embedding record with the 'embedding' vector column. That column is added outside db/schema.rb specifically so schema:load works on Postgres without pgvector (Heroku CI's in-dyno Postgres), so the raw column write raised: ActiveModel::UnknownAttributeError: unknown attribute 'embedding' for Embedding. (surfaced in the ops thread via /app/lib/stacks/etl/connector.rb:54 → etl.rake:38 → test/lib/tasks/etl_rake_test.rb:37). Fix: stub Stacks::Etl::Groups::Connector#new/#run in the sync_all test, mirroring how the Meet sweeps are already stubbed via Stacks::Etl::Meet.expects(:sweep_all_users!) in the same test — so the test asserts sync_all's three-source wiring/order without touching Google or Embedding at all.
…youts # Conflicts: # lib/stacks/task_builder.rb
…ice_tracker fixtures ContributorPayout.created_by_id and invoice_tracker_id are NOT NULL columns (db/schema.rb). The original test passed an unsaved InvoiceTracker.new and never set created_by, so ContributorPayout.create! raised ActiveRecord::RecordInvalid in Heroku CI's real Postgres run (this passed local static review but was never actually executed against a DB before merge). Build a real InvoiceTracker via InvoicePass/ForecastClient/QboAccount fixtures (same pattern as test/models/contributor_test.rb's create_payout! helper) and pass created_by: @admin on every payout.
…ds and a non-zero invoice_tracker total The prior fix only added created_by/invoice_tracker to the fixtures but missed two independent bugs that still made the suite fail: 1. QboAccount.create!(name: ..., realm_id: ...) — QboAccount has no `name` column (schema: client_id, client_secret, realm_id, enterprise_id, all null: false). This raised ActiveRecord::UnknownAttributeError before any test ran. Fixed to match the pattern already used elsewhere (e.g. test/integration/ledger_migration_test.rb): QboAccount.create!(enterprise:, client_id:, client_secret:, realm_id:). 2. The invoice_tracker built in setup had no qbo_invoice and no blueprint, so InvoiceTracker#total resolved to 0. ContributorPayout's contributor_payouts_within_seventy_percent validation then rejected every 00 test payout (contributor_pool_sum 100 > max_amount 0 + 1), raising ActiveRecord::RecordInvalid on ContributorPayout.create! in every test. Fixed by giving the tracker a blueprint with a real $10,000 line total, comfortably covering the $100/$200 test payouts. Detector class and task_builder.rb registration were re-verified — the 618f0a3 merge conflict resolution correctly kept UnacceptedPayouts registered exactly once, so no production code changed here.
…youts Picks up affe3a9 (fix(test): RunnMirror past-only fixture is calendar-dependent) and other main commits landed since our last merge at 0f4251e — our branch's stale merge base was carrying a since-fixed RunnMirror test failure that was blocking Heroku CI independent of the unaccepted_payouts work.
…google_groups stub Found while chasing a persistent Heroku CI failure that survived two unaccepted_payouts_test fixes and a merge to pick up a stale RunnMirror test fix — this bug is unrelated to unaccepted_payouts (predates it, from commit 591b593) but is part of this PR's diff and was very likely the actual root cause blocking CI on every prior push. Verified by actually running the suite locally (Ruby 3.1.7 + Postgres, matching Gemfile exactly), not just static reading: 1. has_entry(:admin_email) is invalid Mocha API usage — has_entry requires either has_entry(key, value) or has_entry(hash); a bare Symbol makes Mocha's parse_option raise 'ArgumentError: Argument is not a Hash' the instant the expectation is declared, unconditionally, regardless of credentials or environment. Fixed to has_key(:admin_email), which is exactly the 'key present, any value' matcher this test wants. 2. Once that was fixed, a second bug surfaced: the two .in_sequence(seq) expectations for the google_groups stub were declared in the wrong order — groups_connector.expects(:run) came BEFORE Stacks::Etl::Groups::Connector.expects(:new) in the source. Mocha's sequences require expectations to be satisfied in DECLARATION order, but the real code (lib/tasks/etl.rake#sync_google_groups) can only ever call .new before calling .run on the object .new returns — so the one possible real call order was reported as 'invoked out of order'. Fixed by declaring Connector.expects(:new) before groups_connector.expects(:run), matching the real call order. Confirmed test/lib/tasks/etl_rake_test.rb now passes (3 runs, 0 failures, 0 errors) locally.
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
1 participant
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
Adds a new admin-task detector: contributors with unaccepted
ContributorPayoutrows (accepted_at: nil) now surface a task nudging them to accept, linking to their ledger page.Discoveries::UnacceptedPayouts— queriesContributorPayout.where(accepted_at: nil), groups by contributor, emits one task per contributor even with multiple unaccepted payouts.contributor.forecast_person.admin_user, falling back to the admin team per the existingBase#taskinvariant.StacksTaskextended with aledgerattribute so aContributorsubject'surlresolves toadmin_contributor_path(contributor, ledger: ledger.id)— matching the existing ledger-tab link pattern atapp/admin/contributors.rb:263-275.TaskBuilder::DISCOVERY_CLASSES.Tests
Added
lib/stacks/task_builder/discoveries/unaccepted_payouts_test.rbcovering:unaccepted_payoutstask, url pointing at their ledger.bin/rails testcould not be executed here. Please run CI / the relevant spec file before merge.Requested by Hugh in Twist (#Operations, "Low priority Stacks requests").