feat(activity): user and entity activity queries - #5566
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdded activity read models with forward-tolerant action decoding and PostgreSQL pagination. Added GraphQL activity events, typed actions, authenticated feeds, and lazy Soup entity activity loading. Propagated activity readers through schema construction and document storage service wiring. Added GraphQL schema declarations, web queries, batching, cursor handling, and tests. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview: https://synoet-activity-graphql-4tmhzn.preview.macro.com/app (999e56f) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (10)
apps/web/src/lib/service-clients/service-storage/graphql/activity.graphql (1)
24-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
EntityActivityFieldsdepends on a variable declared by its caller.The fragment reads
$limit, but the fragment itself cannot declare it. The document is valid because onlyEntityActivityspreads this fragment and declares$limit. If another operation later spreadsEntityActivityFieldswithout declaring$limit, validation fails. Movingactivity(limit: $limit)into the operation selection set removes that coupling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/service-clients/service-storage/graphql/activity.graphql` around lines 24 - 30, Update the EntityActivityFields fragment to stop referencing the caller-provided $limit variable by removing the limit argument from its activity selection. Move activity(limit: $limit) into the EntityActivity operation’s selection set while preserving the existing ...ActivityEventFields selection.crates/graphql_common/src/limit.rs (1)
3-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReturn
NonZeroU32to carry the positivity guarantee to callers.The function already proves
limit > 0, then discards that fact and returnsu32.ActivityReads::subject_feedrequiresNonZeroU32, so each caller must re-derive the invariant. ReturningNonZeroU32moves the guarantee into the type and removes the unreachableexpect.♻️ Proposed refactor
-pub fn parse_limit(limit: Option<i32>, default: i32, max: i32) -> async_graphql::Result<u32> { +pub fn parse_limit( + limit: Option<i32>, + default: i32, + max: i32, +) -> async_graphql::Result<std::num::NonZeroU32> { let limit = limit.unwrap_or(default); if limit <= 0 { return Err(async_graphql::Error::new("limit must be positive")); } if limit > max { return Err(async_graphql::Error::new(format!( "limit must not exceed {max}" ))); } - Ok(u32::try_from(limit).expect("positive GraphQL Int fits in u32")) + let limit = u32::try_from(limit).expect("positive GraphQL Int fits in u32"); + std::num::NonZeroU32::new(limit) + .ok_or_else(|| async_graphql::Error::new("limit must be positive")) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/graphql_common/src/limit.rs` around lines 3 - 14, Update parse_limit to return NonZeroU32, constructing it after validating the positive and maximum bounds so the positivity guarantee is preserved in the type. Remove the now-unnecessary u32 conversion expect, and update callers such as ActivityReads::subject_feed to use the returned NonZeroU32 directly without re-validating or reconstructing it.crates/activity/src/domain/ports.rs (1)
69-75: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse
NonZeroU32forper_entity_limitas well.
subject_feedenforces a nonzero limit by type.entity_activityacceptsu32. A zero value reaches the adapter as SQLLIMIT 0, so every requested entity returns no rows and is absent from the map with no error. The same type guarantee removes that silent case.♻️ Proposed signature change
fn entity_activity( &self, keys: &[(EntityType, String)], - per_entity_limit: u32, + per_entity_limit: NonZeroU32, ) -> impl Future<Output = Result<EntityActivityMap, Self::Err>> + Send;The adapter then uses
i64::from(per_entity_limit.get()).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/activity/src/domain/ports.rs` around lines 69 - 75, Update the entity_activity method’s per_entity_limit parameter to NonZeroU32, matching subject_feed’s nonzero limit contract; preserve the existing return type and behavior, and update adapter implementations and call sites to use per_entity_limit.get() when converting the value for SQL or other numeric APIs.static_assets/schema.graphql (1)
4062-4065: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider making the
inputargument optional.
activity(input: ActivityFeedInput!)requires the argument, but both of its fields are optional:cursor: Stringandlimit: Intat lines 9 and 13. A client fetching the first page with default page size must still writeactivity(input: {}).Compare
messages(offset: Int, limit: Int)at line 3571, which takes no required argument. Givinginputa default of{}would match that ergonomic and remove the empty-object boilerplate. This field is new, so the change is cheap now and breaking after clients adopt it.Make the change in the Rust resolver signature and regenerate this file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@static_assets/schema.graphql` around lines 4062 - 4065, Make the new activity field’s input optional by updating its Rust resolver signature to accept an omitted ActivityFeedInput and apply the existing default behavior, then regenerate static_assets/schema.graphql so activity no longer requires input and clients can query the first page without an empty object.crates/graphql_activity/src/loaders.rs (2)
168-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the distinct-key precondition, because
removeis destructive.Line 172 calls
by_entity.remove(&entity). If two keys in the same limit group resolve to the same(entity_type, entity_id)pair, the first key takes the records and every later duplicate falls through tounwrap_or_default()and receives an empty list.The
DataLoaderpath is safe today:ActivityEdgeKeyderivesHashandEqat line 32, soDataLoaderdeduplicates keys before it callsload, and distinct keys within one limit group imply distinct entities. ButSoupActivityEdgeReader::entity_activityis a public trait method that accepts an arbitraryVec<ActivityEdgeKey>. A direct caller that passes duplicates gets silently wrong results.State the precondition on the trait method so future callers do not trip on it.
♻️ Suggested doc change on the trait method
pub trait SoupActivityEdgeReader: Send + Sync + 'static { - /// Load the newest activity for each requested entity. + /// Load the newest activity for each requested entity. + /// + /// `keys` must be distinct. Implementations may consume their per-entity + /// result on first match, so a repeated key yields an empty list for + /// every occurrence after the first. The `DataLoader` path satisfies + /// this by deduplicating on [`ActivityEdgeKey`]'s `Hash`/`Eq`. fn entity_activity( &self, keys: Vec<ActivityEdgeKey>, ) -> impl Future<Output = HashMap<ActivityEdgeKey, ActivityEdgeLoad>> + Send; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/graphql_activity/src/loaders.rs` around lines 168 - 181, Document the distinct-entity precondition on the public SoupActivityEdgeReader::entity_activity trait method: callers must provide keys resolving to unique (entity_type, entity_id) pairs within each limit group because the loader consumes grouped records with by_entity.remove. Keep the existing loading behavior unchanged.
11-24: 🚀 Performance & Scalability | 🔵 TrivialNote the row ceiling per SQL round trip.
ACTIVITY_EDGE_BATCH_KEYSis 500 andMAX_ACTIVITY_EDGE_LIMITis 100, so one lateral-join query can return up to 50,000 rows. WithMAX_CONCURRENT_LIMIT_GROUPSat 4, up to 200,000ActivityRecordvalues can be materialized concurrently per batch. Each record holds twoStringfields plus an optionalserde_json::Value.Reaching that ceiling requires an operation that selects 500 distinct entities with
limit: 100, so this is not a likely path. Add a metric on rows returned per activity batch so you can observe the real distribution before it becomes a memory problem.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/graphql_activity/src/loaders.rs` around lines 11 - 24, Add instrumentation to the activity batch-loading flow that records the number of rows returned by each SQL batch, using the existing metrics mechanism and a clearly named activity-batch row metric. Measure the combined rows materialized for the batch after the concurrent limit-group queries complete, and preserve the current batching and concurrency behavior.crates/activity/src/outbound/pg_activity_repo/test.rs (1)
326-373: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThis test passes under both cursor derivations, so it does not pin the invariant.
The first raw page holds
doc-new(t+2s) and the corrupt row (t+1s). If the repository derivednextfrom the last decoded record instead of the last raw row,nextwould point atdoc-new. The second page would then return the corrupt row anddoc-old, decode to one record, and still assertlen() == 1withnext == None. Both derivations satisfy every assertion here.The invariant documented at
crates/graphql_activity/src/loaders.rslines 68-71 states that skipped corrupt rows never end the feed. The failing case for a decoded-derived cursor is a page where every raw row is corrupt: the page returns zero records, and a decoded-derivednextwould beNone, ending the feed while valid rows remain behind it. Add that case.💚 Suggested additional test
#[sqlx::test(migrator = "MACRO_DB_MIGRATIONS")] async fn an_all_corrupt_page_still_carries_a_cursor(pool: PgPool) { let repo = PgActivityRepo::new(pool.clone()); let t = base_time(); // Two corrupt rows newest-first, one valid row behind them. With limit 2 // the first page decodes to nothing and must still carry a cursor. for (id, offset) in [(60u128, 2i64), (61, 1)] { sqlx::query!( r#" INSERT INTO activity_events (id, actor_id, subject_id, action, action_payload, entity_type, entity_id, occurred_at) VALUES ($1, 'garbled', 'macro|actor@example.com', 'edited', NULL, 'document', 'doc-corrupt', $2) "#, Uuid::from_u128(id), t + chrono::Duration::seconds(offset), ) .execute(&pool) .await .unwrap(); } repo.insert_activities(&[seed_at(62, CommonAction::Edited, "doc-old", t)]) .await .unwrap(); let first = repo .subject_feed("macro|actor@example.com", None, nz(2)) .await .unwrap(); assert!(first.records.is_empty(), "both raw rows are corrupt"); let next = first .next .expect("an all-corrupt page must still carry a cursor"); let second = repo .subject_feed("macro|actor@example.com", Some(next), nz(2)) .await .unwrap(); assert_eq!(second.records.len(), 1); assert_eq!(second.records[0].entity_id, "doc-old"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/activity/src/outbound/pg_activity_repo/test.rs` around lines 326 - 373, Add an all-corrupt-page test alongside a_corrupt_row_shrinks_the_page_but_never_ends_pagination: insert two newest corrupt rows and one older valid row, fetch with limit 2, assert the first page has no records but still provides next, then fetch with that cursor and assert the older valid record is returned. This must verify pagination advances from the last raw row rather than the last decoded record.crates/graphql_activity/src/objects/test.rs (1)
39-66: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for the two participant variants.
ParticipantAddedandParticipantRemovedhave separate mapping bodies atcrates/graphql_activity/src/objects.rslines 162-171. Both extract the principal string the same way, and neither is exercised here. A swap between the two arms would compile and pass the current suite.
type_nameat lines 86-100 forces compile-time exhaustiveness over the union, but it does not verify that eachRecordedActionmaps to the matching member.💚 Suggested additional test
#[test] fn participant_changes_keep_their_direction() { let change = || activity::ParticipantChange { participant: actor(), }; let added = GraphqlActivityEvent::from(record(RecordedAction::Known( Action::ParticipantAdded(change()), ))); match added.action { GraphqlActivityAction::ParticipantAdded(p) => { assert_eq!(p.participant, "macro|teo@example.com"); } other => panic!("expected ParticipantAdded, got {}", type_name(&other)), } let removed = GraphqlActivityEvent::from(record(RecordedAction::Known( Action::ParticipantRemoved(change()), ))); match removed.action { GraphqlActivityAction::ParticipantRemoved(p) => { assert_eq!(p.participant, "macro|teo@example.com"); } other => panic!("expected ParticipantRemoved, got {}", type_name(&other)), } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/graphql_activity/src/objects/test.rs` around lines 39 - 66, Add a test alongside payload_actions_carry_their_payload_fields that constructs RecordedAction::Known variants for both Action::ParticipantAdded and Action::ParticipantRemoved, converts each with GraphqlActivityEvent::from, and matches the corresponding GraphqlActivityAction variant. Assert each extracted participant equals the actor fixture value, while retaining the existing unexpected-variant type_name panic.crates/graphql_activity/src/objects.rs (1)
79-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReconsider the
nothingfiller field before the schema ships.GraphQL requires at least one field per object type, so a filler field is necessary for the six payload-free actions. The current shape has two problems.
First, the doc comment at line 84 is published verbatim to clients. It appears six times in
static_assets/schema.graphqlat lines 205, 215, 225, 274, 284, and 346 as the documentation fornothing: Boolean!. It is lowercase, ungrammatical, and describes an internal constraint rather than the field's meaning.Second,
nothing: Boolean!is a permanent non-null field in the public contract. It always resolves tofalse, because lines 149-154 build these objects withDefault::default(). Removing it later is a breaking schema change.Consider making the filler carry real information instead, for example the action tag. Clients could then discriminate without depending on
__typename, and the field would justify its place in the contract. If you keep the boolean, at minimum rewrite the doc comment so it reads as client-facing documentation.♻️ Minimum change: rewrite the published doc comment
macro_rules! payload_free_action_objects { ($($(#[$doc:meta])* $name:ident),+ $(,)?) => {$( $(#[$doc])* #[derive(Default, SimpleObject)] pub struct $name { - /// this object has nothing as a field but we need at least 1 field + /// Always `false`. This action carries no payload; the field + /// exists because a GraphQL object requires at least one field. nothing: bool, } )+}; }Regenerate
static_assets/schema.graphqlafter any change here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/graphql_activity/src/objects.rs` around lines 79 - 103, Update the filler field in the payload_free_action_objects! macro so its published documentation is clear, grammatical, and describes the client-facing meaning rather than the internal GraphQL constraint; preserve the existing boolean field behavior. Regenerate static_assets/schema.graphql so the updated documentation appears for all six generated payload types.crates/graphql_activity/src/feed/test.rs (1)
10-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove or validate the serialized cursor limit.
encode_cursorserializeslimit, butdecode_cursorandActivityFeedReaderuse only the keyset position and current request limit. Use a feed-specific payload withoutlimit, or validate it during decoding.Add a test for valid base64 that contains the wrong JSON shape, such as
{}.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/graphql_activity/src/feed/test.rs` around lines 10 - 13, Update the cursor handling around encode_cursor and decode_cursor so the serialized limit is either removed in favor of a feed-specific payload containing only the keyset position, or validated during decoding before accepting the cursor. Add a decode_cursor test using valid base64 with an invalid JSON shape such as {} and assert that decoding rejects it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/lib/service-clients/service-storage/graphql/activity.graphql`:
- Around line 40-60: Update the action selection in the GraphQL activity query
to add empty `nothing` fragments for GraphqlActivityCreated,
GraphqlActivityEdited, GraphqlActivityOpened, GraphqlActivityDeleted,
GraphqlActivityMessaged, and GraphqlActivitySent, ensuring every typed union
member selects its required field while preserving the existing fields for other
action types.
In `@crates/activity/src/outbound/pg_activity_repo.rs`:
- Around line 237-250: Deduplicate the `(EntityType, String)` pairs at the start
of `entity_activity` before constructing `entity_types` and `entity_ids`.
Preserve the existing empty-input behavior, then build both UNNEST arrays from
the unique keys so each entity is queried and grouped once while retaining the
per-entity activity limit.
In `@crates/activity/src/outbound/pg_activity_repo/test.rs`:
- Around line 260-274: Regenerate the SQLx offline metadata for the new queries
in the activity repository tests by running `nix develop --command just
prepare_db`, then commit the generated entries under the root `.sqlx/`
directory. Do not manually edit any `.sqlx/query-*.json` files.
In `@crates/complete_graph/src/schema/test.rs`:
- Around line 544-551: Update the record filter in the subject_feed fixture
around the page collection to require record.subject_id to match the requested
subject_id before applying the cursor condition. Preserve the existing
occurred_at/id cursor pagination behavior for matching records.
In `@crates/graphql_activity/src/objects.rs`:
- Around line 110-113: Update the `to` field documentation in `objects.rs` and
the corresponding schema documentation in `static_assets/schema.graphql` to
state that `null` represents both an absent value and a cleared value. Do not
claim clients can distinguish these cases unless the field representation is
changed.
In `@static_assets/schema.graphql`:
- Around line 3618-3621: Update the shared activity contract by extending the
Rust documentation for the activity field in the relevant edges definition with
its batching and outage behavior, then regenerate static_assets/schema.graphql.
Add "activity" to the shared-field assertions in the SDL test so the interface
contract is validated.
- Around line 254-258: Update the GraphqlActivityEvent entityType field to use
GraphqlSoupEntityType for Soup-backed activity references, or add explicit
resolution for USER, TEAM, STATIC_FILE, CRM_CONTACT, and SKILL before exposing
them. Ensure activity entity types remain consistent with the entity type
expected by Soup.
---
Nitpick comments:
In `@apps/web/src/lib/service-clients/service-storage/graphql/activity.graphql`:
- Around line 24-30: Update the EntityActivityFields fragment to stop
referencing the caller-provided $limit variable by removing the limit argument
from its activity selection. Move activity(limit: $limit) into the
EntityActivity operation’s selection set while preserving the existing
...ActivityEventFields selection.
In `@crates/activity/src/domain/ports.rs`:
- Around line 69-75: Update the entity_activity method’s per_entity_limit
parameter to NonZeroU32, matching subject_feed’s nonzero limit contract;
preserve the existing return type and behavior, and update adapter
implementations and call sites to use per_entity_limit.get() when converting the
value for SQL or other numeric APIs.
In `@crates/activity/src/outbound/pg_activity_repo/test.rs`:
- Around line 326-373: Add an all-corrupt-page test alongside
a_corrupt_row_shrinks_the_page_but_never_ends_pagination: insert two newest
corrupt rows and one older valid row, fetch with limit 2, assert the first page
has no records but still provides next, then fetch with that cursor and assert
the older valid record is returned. This must verify pagination advances from
the last raw row rather than the last decoded record.
In `@crates/graphql_activity/src/feed/test.rs`:
- Around line 10-13: Update the cursor handling around encode_cursor and
decode_cursor so the serialized limit is either removed in favor of a
feed-specific payload containing only the keyset position, or validated during
decoding before accepting the cursor. Add a decode_cursor test using valid
base64 with an invalid JSON shape such as {} and assert that decoding rejects
it.
In `@crates/graphql_activity/src/loaders.rs`:
- Around line 168-181: Document the distinct-entity precondition on the public
SoupActivityEdgeReader::entity_activity trait method: callers must provide keys
resolving to unique (entity_type, entity_id) pairs within each limit group
because the loader consumes grouped records with by_entity.remove. Keep the
existing loading behavior unchanged.
- Around line 11-24: Add instrumentation to the activity batch-loading flow that
records the number of rows returned by each SQL batch, using the existing
metrics mechanism and a clearly named activity-batch row metric. Measure the
combined rows materialized for the batch after the concurrent limit-group
queries complete, and preserve the current batching and concurrency behavior.
In `@crates/graphql_activity/src/objects.rs`:
- Around line 79-103: Update the filler field in the
payload_free_action_objects! macro so its published documentation is clear,
grammatical, and describes the client-facing meaning rather than the internal
GraphQL constraint; preserve the existing boolean field behavior. Regenerate
static_assets/schema.graphql so the updated documentation appears for all six
generated payload types.
In `@crates/graphql_activity/src/objects/test.rs`:
- Around line 39-66: Add a test alongside
payload_actions_carry_their_payload_fields that constructs RecordedAction::Known
variants for both Action::ParticipantAdded and Action::ParticipantRemoved,
converts each with GraphqlActivityEvent::from, and matches the corresponding
GraphqlActivityAction variant. Assert each extracted participant equals the
actor fixture value, while retaining the existing unexpected-variant type_name
panic.
In `@crates/graphql_common/src/limit.rs`:
- Around line 3-14: Update parse_limit to return NonZeroU32, constructing it
after validating the positive and maximum bounds so the positivity guarantee is
preserved in the type. Remove the now-unnecessary u32 conversion expect, and
update callers such as ActivityReads::subject_feed to use the returned
NonZeroU32 directly without re-validating or reconstructing it.
In `@static_assets/schema.graphql`:
- Around line 4062-4065: Make the new activity field’s input optional by
updating its Rust resolver signature to accept an omitted ActivityFeedInput and
apply the existing default behavior, then regenerate
static_assets/schema.graphql so activity no longer requires input and clients
can query the first page without an empty object.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c7c30d3-0ea6-416b-820e-edb1a0d8dd57
⛔ Files ignored due to path filters (5)
.sqlx/query-283b0b2adabca5459246bb424a01fb7cfa158c6a310ecffd52fbba9ec1ce6694.jsonis excluded by!**/.sqlx/**.sqlx/query-2c85606cfd85f04433c60549ff5332d854fa30f8fb472937be7c78d9a8a83290.jsonis excluded by!**/.sqlx/**.sqlx/query-72f271d631f1aed6643a5f5ab371dcc65982ba1efdfd0f9b3c43c3336a965dd5.jsonis excluded by!**/.sqlx/**Cargo.lockis excluded by!**/*.lock,!**/Cargo.lockapps/web/src/lib/service-clients/service-storage/graphql/generated/graphql.tsis excluded by!**/generated/**,!apps/web/src/lib/service-clients/**/generated/**
📒 Files selected for processing (30)
.github/workspace-dep-closures.jsonCargo.tomlapps/web/src/lib/service-clients/service-storage/graphql/activity.graphqlcrates/activity/Cargo.tomlcrates/activity/src/domain/models.rscrates/activity/src/domain/models/test.rscrates/activity/src/domain/ports.rscrates/activity/src/lib.rscrates/activity/src/outbound/pg_activity_repo.rscrates/activity/src/outbound/pg_activity_repo/test.rscrates/complete_graph/Cargo.tomlcrates/complete_graph/src/edges.rscrates/complete_graph/src/lib.rscrates/complete_graph/src/schema.rscrates/complete_graph/src/schema/test.rscrates/graphql_activity/Cargo.tomlcrates/graphql_activity/src/feed.rscrates/graphql_activity/src/feed/test.rscrates/graphql_activity/src/lib.rscrates/graphql_activity/src/loaders.rscrates/graphql_activity/src/objects.rscrates/graphql_activity/src/objects/test.rscrates/graphql_common/src/lib.rscrates/graphql_common/src/limit.rscrates/graphql_entity_mutation/src/mutations/test.rscrates/graphql_soup/src/objects.rsservices/document_storage_service/src/api/context.rsservices/document_storage_service/src/api/graphql_soup.rsservices/document_storage_service/src/main.rsstatic_assets/schema.graphql
- entity_activity dedupes its keys: a repeated (type, id) pair would run its lateral twice and stack 2x the limit into one map entry (test extended with a duplicate key) - sdl contract test asserts activity on the shared interface and the document object, like the other shared edges - the feed test fake scopes records by subject_id like the real repo - PropertyChanged.to doc no longer promises a cleared/null distinction the wire cannot carry
synoet
changed the title
Action::from_columns is the promised inverse of to_columns; unknown tags surface as RecordedAction::Unknown so rows written by newer deployments read forward-tolerantly instead of failing pages. New ActivityReads port on PgActivityRepo: keyset-paginated subject feed (two static queries so the index stays usable) and a batched UNNEST+LATERAL per-entity timeline for the GraphQL DataLoader.
GraphqlActivityEvent carries entity references only (clients resolve names from the normalized Soup cache); the action is a typed union with an Unknown fallback member so newer-vocabulary rows degrade instead of erroring. Edge loads batch through a DataLoader with per-operation cost caps; the feed reader keyset-paginates behind an opaque cursor.
Threads one new reader generic (AcR) through the schema. The edge is a field on the shared SoupEdges object, so every entity type gets it; the recording-reader test pins that an unselected edge never touches the reader and a selected one batches across entities in one call. user.activity keyset-paginates behind an opaque cursor and includes delegated actions (subject = the viewer's principal).
Reads go through the readonly pool; the Kafka consumer's writer-pool repo is unchanged. Also aliases the ActivityReads entity map to keep clippy's type-complexity lint happy, and regenerates the web GraphQL client + workspace dep closures.
Correctness: - Feed pagination probe moves into the repo and works on raw rows: ActivityReads::subject_feed returns an ActivityFeedPage whose next cursor is derived before decode-skipping, so a corrupt row shrinks a page but can never end the feed (new test pins this). - Edge cost caps are now genuinely per-operation (no max_batch_size re-chunking) and an over-budget batch degrades to empty timelines with a warn instead of erroring the whole Soup response; the loader error type is gone (Infallible). - The entity batch query gets an outer ORDER BY so newest-first per entity no longer depends on the planner preserving the lateral sort. Cleanup: - from_columns parses tags via a strum-derived ActionTag discriminant enum: a new Action variant now fails compilation until decoded. - Payload decode borrows (&Value) instead of cloning per row. - graphql_common::parse_limit replaces the copy-pasted validators. - Per-limit edge queries run concurrently (join_all). - Payload-free union members collapse into one macro.
Declares activity as a GraphqlSoupEntity interface field (new SoupEntityEdges::ActivityEvent associated type + resolve_activity), so clients can select it on the interface like notifications instead of repeating 11 inline fragments. Adds the web documents: MyActivity (cursor feed) and EntityActivity (soup-filtered edge), sharing an ActivityEventFields fragment with typed action selections.
…eed limit The cap design refused schema-valid queries (500-entity page x limit 11 exceeded the row budget) and did it silently, killing legitimate keys that coalesced into the same batch — and the caps were per-wave anyway (async-graphql's default max_batch_size is 1000). Every schema-valid selection is now served: per-entity work stays bounded by the limit argument's validation, and max_batch_size(500) only sizes each SQL round trip. Per-limit query groups run with bounded concurrency (buffer_unordered) so aliased limits can't drain the pool. subject_feed takes NonZeroU32 — a zero-row page can't carry a next position, so the state is now unrepresentable. Also dedupes the email limit validator onto graphql_common::parse_limit and unlinks the pub(crate) ActionTag doc reference.
- entity_activity dedupes its keys: a repeated (type, id) pair would run its lateral twice and stack 2x the limit into one map entry (test extended with a duplicate key) - sdl contract test asserts activity on the shared interface and the document object, like the other shared edges - the feed test fake scopes records by subject_id like the real repo - PropertyChanged.to doc no longer promises a cleared/null distinction the wire cannot carry
4735abe to
f1af7da
Compare
|
|
||
| /// Decode an opaque feed cursor into its keyset position. | ||
| fn decode_cursor(cursor: String) -> async_graphql::Result<(DateTime<Utc>, Uuid)> { | ||
| let cursor = Base64Str::<ActivityFeedCursor>::new_from_string(cursor) |
There was a problem hiding this comment.
It might be useful to reach for the built in cursor behaviour if it fits here. https://async-graphql.github.io/async-graphql/en/cursor_connections.html
2 participants
Uh oh!
There was an error while loading. Please reload this page.