Skip to content

Automation should use event store#1962

Merged
alex-Arc merged 2 commits intomasterfrom
automation-should-use-event-store
Mar 2, 2026
Merged

Automation should use event store#1962
alex-Arc merged 2 commits intomasterfrom
automation-should-use-event-store

Conversation

@alex-Arc
Copy link
Copy Markdown
Collaborator

@alex-Arc alex-Arc commented Feb 3, 2026

No description provided.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 3, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch automation-should-use-event-store

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@alex-Arc alex-Arc force-pushed the automation-should-use-event-store branch from 1acef21 to 76df53f Compare February 17, 2026 06:33
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
apps/server/src/api-data/automation/automation.dao.ts (1)

16-39: Preserve RuntimeStore fields when state is provided.

When state is passed, the current construction enumerates specific fields and drops any other RuntimeStore keys that exist now or may be added later. Consider starting from a polled snapshot and overriding the state-derived fields to keep parity.

♻️ Suggested refactor
 export function getRuntimeStore(state?: RuntimeState): RuntimeStore {
-  if (!state) {
-    return eventStore.poll();
-  }
-  const { message, auxtimer1, auxtimer2, auxtimer3, ping } = eventStore.poll();
+  const store = eventStore.poll();
+  if (!state) {
+    return store;
+  }
   const { clock, timer, rundown, offset, eventNow, eventNext, eventFlag, groupNow } = state;
   return {
+    ...store,
     clock,
     timer,
-    message,
     rundown,
     offset,
     eventNow,
     eventNext,
     eventFlag,
     groupNow,
-    auxtimer1,
-    auxtimer2,
-    auxtimer3,
-    ping,
   };
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/server/src/api-data/automation/automation.dao.ts` around lines 16 - 39,
getRuntimeStore currently builds the returned RuntimeStore from scratch when a
RuntimeState is provided, which drops any extra keys from the polled snapshot;
instead call eventStore.poll() first and merge/override only the state-driven
fields so all existing and future RuntimeStore keys are preserved. Modify
getRuntimeStore to obtain a polled snapshot via eventStore.poll(), then return
an object that spreads that snapshot and assigns/overrides the fields derived
from the passed-in RuntimeState (clock, timer, rundown, offset, eventNow,
eventNext, eventFlag, groupNow) so no polled properties are lost.
apps/server/src/api-data/automation/clients/http.client.ts (1)

11-19: Rename state parameter to store for consistency.

The helper now operates on the store; aligning the parameter name avoids confusion.

✏️ Suggested rename
-function preparePayload(output: HTTPOutput, state: DeepReadonly<RuntimeStore>): string {
-  const parsedUrl = parseTemplateNested(output.url, state);
+function preparePayload(output: HTTPOutput, store: DeepReadonly<RuntimeStore>): string {
+  const parsedUrl = parseTemplateNested(output.url, store);
   return parsedUrl;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/server/src/api-data/automation/clients/http.client.ts` around lines 11 -
19, The preparePayload helper uses a parameter named state but callers and
surrounding code use store; rename the function parameter from state to store in
the preparePayload declaration and any internal references so it matches
emitHTTP’s call signature and naming conventions (function
preparePayload(output: HTTPOutput, store: DeepReadonly<RuntimeStore>) and usages
inside preparePayload such as parseTemplateNested(output.url, store)). Ensure no
other references to the old parameter name remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/server/src/api-data/automation/automation.dao.ts`:
- Around line 16-39: getRuntimeStore currently builds the returned RuntimeStore
from scratch when a RuntimeState is provided, which drops any extra keys from
the polled snapshot; instead call eventStore.poll() first and merge/override
only the state-driven fields so all existing and future RuntimeStore keys are
preserved. Modify getRuntimeStore to obtain a polled snapshot via
eventStore.poll(), then return an object that spreads that snapshot and
assigns/overrides the fields derived from the passed-in RuntimeState (clock,
timer, rundown, offset, eventNow, eventNext, eventFlag, groupNow) so no polled
properties are lost.

In `@apps/server/src/api-data/automation/clients/http.client.ts`:
- Around line 11-19: The preparePayload helper uses a parameter named state but
callers and surrounding code use store; rename the function parameter from state
to store in the preparePayload declaration and any internal references so it
matches emitHTTP’s call signature and naming conventions (function
preparePayload(output: HTTPOutput, store: DeepReadonly<RuntimeStore>) and usages
inside preparePayload such as parseTemplateNested(output.url, store)). Ensure no
other references to the old parameter name remain.

@alex-Arc alex-Arc force-pushed the automation-should-use-event-store branch from 76df53f to b72e61a Compare February 23, 2026 17:01
Comment thread apps/server/src/api-data/automation/clients/http.client.ts Outdated
Comment thread apps/server/src/api-data/automation/automation.dao.ts Outdated
Comment thread apps/server/src/api-data/automation/automation.service.ts Outdated
Comment thread apps/server/src/api-data/automation/automation.dao.ts Outdated
Comment thread apps/server/src/api-data/automation/automation.service.ts Outdated
@alex-Arc alex-Arc force-pushed the automation-should-use-event-store branch 3 times, most recently from c29e2f7 to bae9daf Compare March 2, 2026 11:03
@alex-Arc alex-Arc force-pushed the automation-should-use-event-store branch from bae9daf to cfc7b58 Compare March 2, 2026 11:08
@alex-Arc alex-Arc requested a review from cpvalente March 2, 2026 11:12
@alex-Arc alex-Arc merged commit 358ad79 into master Mar 2, 2026
4 checks passed
@alex-Arc alex-Arc deleted the automation-should-use-event-store branch March 2, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants