Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add relevantBlock function #1100

Merged
merged 26 commits into from
Jul 23, 2024
Merged

add relevantBlock function #1100

merged 26 commits into from
Jul 23, 2024

Conversation

alex-Arc
Copy link
Collaborator

@alex-Arc alex-Arc commented Jun 23, 2024

Adds block info to the runtimeStore

{
"block": {"title":"<Title of the current block>","type":"block","id":"<id of the current block>"} || null,
"startedAt": <time in ms whan thsi block was enterd> || null
}

Copy link
Contributor

coderabbitai bot commented Jun 23, 2024

Walkthrough

The overall changes introduce a currentBlockId property to enhance state management across the application. This addition improves the handling and identification of the current block in various components, including the Cuesheet, hooks, stores, and services. The updates optimize event processing and rendering logic based on the current block's state, resulting in a more responsive and accurate application behavior.

Changes

File / Path Change Summary
apps/client/src/common/hooks/useSocket.ts Added currentBlockId to state in the useCuesheet hook to retrieve id from state.currentBlock.block or default to null.
apps/client/src/common/stores/runtime.ts Enhanced runtimeStorePlaceholder to include a currentBlock object with block and startedAt properties.
apps/client/src/common/utils/socket.ts Introduced a new case 'ontime-currentBlock' in the connectSocket function to update currentBlock in runtime with the provided payload.
apps/client/src/features/cuesheet/Cuesheet.tsx Updated CuesheetProps to include currentBlockId and modified rendering logic based on currentBlockId and isPast values.
apps/client/src/features/cuesheet/CuesheetWrapper.tsx Added currentBlockId to props in the CuesheetWrapper component.
apps/server/src/app.ts Added a currentBlock object, initialized with block and startedAt, to the parameters of the startServer function.
apps/server/src/services/TimerService.ts Updated roll method to accept a single parameter of type OntimeRundown instead of an array of OntimeEvent.
apps/server/src/services/runtime-service/RuntimeService.ts Incorporated getRundown function for various operations, enhancing event handling and state management.
apps/server/src/stores/__tests__/runtimeState.test.ts Added assertions to verify currentBlock.block is null after specific operations in tests.
apps/server/src/stores/runtimeState.ts Enhanced RuntimeState by adding currentBlock state, modifying function signatures to include playableEvents and rundown, and updating logic related to currentBlock management.
packages/utils/src/rundown-utils/rundownUtils.ts Updated imports and added getRelevantBlock, filterPlayable, and filterTimedEvents functions to manage and filter rundown events efficiently.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server
    participant Cuesheet
    participant RuntimeService
    Client->>Server: Connect Socket
    Server->>Cuesheet: Update currentBlockId
    Cuesheet->>RuntimeService: Fetch currentBlock state
    RuntimeService->>Cuesheet: Return currentBlock state
    Cuesheet->>Client: Render based on currentBlockId
Loading

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@alex-Arc alex-Arc linked an issue Jun 25, 2024 that may be closed by this pull request
@alex-Arc alex-Arc marked this pull request as ready for review July 12, 2024 13:27
Copy link
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.

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (2)
apps/server/src/services/TimerService.ts (1)

99-100: Ensure backward compatibility for the roll method.

The roll method signature has been updated to accept playableEvents and rundown. However, not all calls to this method have been updated accordingly. The following locations need to be revised to match the new method signature:

  • apps/server/src/api-integration/integration.controller.ts:
    • Line 1: runtimeService.roll();
  • apps/server/src/services/runtime-service/RuntimeService.ts:
    • Line 1: this.roll();
    • Line 2: this.roll();

Please update these calls to match the new signature to prevent runtime errors.

Analysis chain

Ensure backward compatibility for the roll method.

The roll method signature has been updated to accept playableEvents and rundown. Ensure that all calls to this method are updated accordingly to prevent runtime errors.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `roll` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type js -A 5 $'roll'

Length of output: 24


Script:

#!/bin/bash
# Description: Verify all function calls to `roll` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type js,ts -A 5 'roll('

Length of output: 91


Script:

#!/bin/bash
# Description: Verify all function calls to `roll` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type js,ts -A 5 'roll\('

Length of output: 61


Script:

#!/bin/bash
# Description: Verify all function calls to `roll` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type js -A 5 'roll\('
rg --type ts -A 5 'roll\('

Length of output: 5602

apps/client/src/features/cuesheet/Cuesheet.tsx (1)

94-94: Remove or handle console.log.

The console.log statement for currentBlockId should be removed or handled appropriately before deploying to production.

- console.log(currentBlockId);
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between fb09631 and e36aa4b.

Files ignored due to path filters (4)
  • packages/types/src/definitions/runtime/BlockState.type.ts is excluded by none and included by none
  • packages/types/src/definitions/runtime/RuntimeStore.type.ts is excluded by none and included by none
  • packages/types/src/index.ts is excluded by none and included by none
  • packages/utils/index.ts is excluded by none and included by none
Files selected for processing (12)
  • apps/client/src/common/hooks/useSocket.ts (1 hunks)
  • apps/client/src/common/stores/runtime.ts (1 hunks)
  • apps/client/src/common/utils/socket.ts (1 hunks)
  • apps/client/src/features/cuesheet/Cuesheet.tsx (4 hunks)
  • apps/client/src/features/cuesheet/CuesheetWrapper.tsx (1 hunks)
  • apps/server/src/app.ts (1 hunks)
  • apps/server/src/services/TimerService.ts (2 hunks)
  • apps/server/src/services/runtime-service/RuntimeService.ts (7 hunks)
  • apps/server/src/stores/tests/runtimeState.test.ts (4 hunks)
  • apps/server/src/stores/runtimeState.ts (13 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.test.ts (2 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.ts (2 hunks)
Files skipped from review due to trivial changes (2)
  • apps/client/src/common/hooks/useSocket.ts
  • apps/client/src/common/stores/runtime.ts
Additional comments not posted (24)
apps/client/src/features/cuesheet/CuesheetWrapper.tsx (1)

110-110: LGTM! But ensure currentBlockId is properly passed and utilized.

The currentBlockId prop has been added to the Cuesheet component. Ensure that this prop is correctly passed and utilized within the Cuesheet component.

apps/client/src/common/utils/socket.ts (1)

153-158: LGTM! The new event type ontime-blockState is handled correctly.

The ontime-blockState event is logged, the runtime is patched, and DevTools are updated accordingly.

apps/client/src/features/cuesheet/Cuesheet.tsx (2)

118-119: Ensure currentBlockId logic is correct.

The currentBlockId is used to conditionally render the block rows. Ensure that the logic for using currentBlockId is correct and does not introduce any bugs.


24-24: Ensure currentBlockId is correctly utilized.

The currentBlockId prop has been added to the CuesheetProps interface. Ensure that this prop is correctly passed and utilized within the component.

Verification successful

To ensure that currentBlockId is correctly utilized, let's delve deeper into the implementation details of CuesheetWrapper.tsx and Cuesheet.tsx.


currentBlockId is correctly utilized.

  • currentBlockId is passed as a prop from CuesheetWrapper.tsx to Cuesheet.tsx.
  • Within Cuesheet.tsx, currentBlockId is correctly used in logical checks and rendering.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `currentBlockId` is correctly utilized within the component.

# Test: Search for the usage of `currentBlockId` within the file.
rg --type ts -A 5 $'currentBlockId'

Length of output: 2580


Script:

#!/bin/bash
# Description: Verify the correct usage of `currentBlockId` in `CuesheetWrapper.tsx` and `Cuesheet.tsx`.

# Check the implementation in CuesheetWrapper.tsx
echo "Checking CuesheetWrapper.tsx:"
rg --type ts -A 10 'currentBlockId=' apps/client/src/features/cuesheet/CuesheetWrapper.tsx

# Check the usage in Cuesheet.tsx
echo "Checking Cuesheet.tsx:"
rg --type ts -A 10 'currentBlockId' apps/client/src/features/cuesheet/Cuesheet.tsx

Length of output: 1601

apps/server/src/stores/__tests__/runtimeState.test.ts (3)

97-102: Ensure comprehensive testing of blockState.block.

The test checks that blockState.block is null after calling load. Ensure that all relevant scenarios for blockState.block are covered in tests.


167-172: Verify blockState.block in runtime offset test.

The test correctly verifies that blockState.block is null after calling load. Consider adding tests for non-null scenarios if applicable.


Line range hint 187-204:
Validate blockState.block in next event test.

Ensure that blockState.block is validated correctly in the context of loading the next event.

apps/server/src/app.ts (1)

182-185: Verify initialization of blockState.

The blockState is initialized with block and startedAt properties as null. Ensure this initialization is consistent with the rest of the codebase and the desired functionality.

packages/utils/src/rundown-utils/rundownUtils.test.ts (1)

267-299: Comprehensive testing of relevantBlock.

The tests cover various scenarios for the relevantBlock function. Ensure all edge cases are considered, such as empty or malformed rundown arrays.

apps/server/src/stores/runtimeState.ts (9)

1-11: Add blockState to RuntimeState type.

The addition of blockState to the RuntimeState type appears correct and consistent with the other state properties.

Also applies to: 55-55


72-75: Initialize blockState in runtimeState.

The initialization of blockState within runtimeState is correct and consistent with the rest of the state initialization.


98-99: Reset blockState in clear function.

The clear function correctly resets the blockState properties to null.


152-159: Update load function parameters.

The load function now correctly accepts playableEvents and rundown as parameters.


190-192: Update blockState.block in loadNow function.

The loadNow function correctly updates the blockState.block using the getRelevantBlock function.


294-294: Reset blockState.startedAt in reload function.

The reload function correctly resets the blockState.startedAt property.


305-306: Update reloadAll function parameters.

The reloadAll function now correctly accepts playableEvents and rundown as parameters.


333-335: Update blockState.startedAt in start function.

The start function correctly updates the blockState.startedAt property.


486-491: Update roll function parameters.

The roll function now correctly accepts playableEvents and rundown as parameters.

apps/server/src/services/runtime-service/RuntimeService.ts (6)

104-106: Update checkTimerUpdate function to handle blockState.

The checkTimerUpdate function correctly integrates the logic to handle the blockState property.


238-239: Update maybeUpdate function parameters.

The maybeUpdate function now correctly accepts playableEvents and rundown as parameters.


263-265: Update loadEvent function parameters.

The loadEvent function now correctly accepts playableEvents and rundown as parameters.


514-516: Update roll function parameters.

The roll function now correctly accepts playableEvents and rundown as parameters.


546-548: Update resume function parameters.

The resume function now correctly accepts playableEvents and rundown as parameters.


610-613: Update broadcastResult function to handle blockState.

The broadcastResult function correctly integrates the logic to handle the blockState property.

packages/utils/src/rundown-utils/rundownUtils.ts Outdated Show resolved Hide resolved
@alex-Arc alex-Arc requested a review from cpvalente July 12, 2024 13:41
packages/utils/src/rundown-utils/rundownUtils.ts Outdated Show resolved Hide resolved
packages/utils/src/rundown-utils/rundownUtils.ts Outdated Show resolved Hide resolved
apps/client/src/common/utils/socket.ts Outdated Show resolved Hide resolved
apps/client/src/common/stores/runtime.ts Outdated Show resolved Hide resolved
apps/server/src/services/TimerService.ts Outdated Show resolved Hide resolved
Copy link
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.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e36aa4b and 1aaa07b.

Files ignored due to path filters (3)
  • packages/types/src/definitions/runtime/CurrentBlockState.type.ts is excluded by none and included by none
  • packages/types/src/definitions/runtime/RuntimeStore.type.ts is excluded by none and included by none
  • packages/types/src/index.ts is excluded by none and included by none
Files selected for processing (8)
  • apps/client/src/common/hooks/useSocket.ts (1 hunks)
  • apps/client/src/common/stores/runtime.ts (1 hunks)
  • apps/client/src/common/utils/socket.ts (1 hunks)
  • apps/server/src/app.ts (1 hunks)
  • apps/server/src/services/runtime-service/RuntimeService.ts (7 hunks)
  • apps/server/src/stores/tests/runtimeState.test.ts (4 hunks)
  • apps/server/src/stores/runtimeState.ts (13 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.ts (2 hunks)
Files skipped from review as they are similar to previous changes (5)
  • apps/client/src/common/hooks/useSocket.ts
  • apps/client/src/common/stores/runtime.ts
  • apps/client/src/common/utils/socket.ts
  • apps/server/src/app.ts
  • apps/server/src/stores/runtimeState.ts
Additional context used
GitHub Check: unit-test
apps/server/src/stores/__tests__/runtimeState.test.ts

[failure] 171-171: src/stores/tests/runtimeState.test.ts > mutation on runtimeState > playback operations > runtime offset
AssertionError: expected +0 to be 1500 // Object.is equality

  • Expected
  • Received
  • 1500
  • 0

❯ src/stores/tests/runtimeState.test.ts:171:43

Additional comments not posted (10)
apps/server/src/stores/__tests__/runtimeState.test.ts (4)

102-102: Approve the new assertion for newState.blockState.block.

The addition of this assertion is a good practice to ensure that the block state is correctly handled. It helps maintain robustness in state management by explicitly checking expected values.


172-172: Approve the new assertions for newState.currentBlock.block.

These assertions are crucial for verifying that the block state is correctly reset or initialized in different test scenarios. They help ensure the robustness and correctness of the state management.

Also applies to: 204-204


97-97: Verify the new parameter in the load function call.

The load function now includes an additional parameter. Ensure that this change is consistent with the updated function signature and that the new parameter is correctly utilized in the context of this test.

Verification successful

The new parameter in the load function call is correctly utilized.

The load function definition in apps/server/src/stores/runtimeState.ts includes four parameters, and the test file apps/server/src/stores/__tests__/runtimeState.test.ts has been updated consistently to reflect this change.

  • apps/server/src/stores/runtimeState.ts (function definition)
  • apps/server/src/stores/__tests__/runtimeState.test.ts (function usage)
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the new parameter usage in the `load` function.
# Test: Search for the `load` function definition and usage. Expect: Consistent usage of the new parameter across the codebase.
rg --type python -A 5 $'load'

Length of output: 59


Script:

#!/bin/bash
# Description: Verify the new parameter usage in the `load` function.
# Test: Search for the `load` function definition and usage in TypeScript files. Expect: Consistent usage of the new parameter across the codebase.
rg --type ts -A 5 'load'

Length of output: 208570


167-167: Verify the updated load function call in another test context.

Ensure that the additional parameter in the load function call is correctly used and aligns with the updated function signature, similar to the previous change.

Verification successful

Verify the updated load function call in another test context.

Ensure that the additional parameter in the load function call is correctly used and aligns with the updated function signature, similar to the previous change.

  • apps/server/src/stores/runtimeState.ts: Definition of load function.
  • apps/server/src/stores/__tests__/runtimeState.test.ts: Usage of load function in the test context.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the updated `load` function call in another test context.
# Test: Search for the `load` function definition and usage. Expect: Consistent usage of the new parameter across different test cases.
rg --type python -A 5 $'load'

Length of output: 59


Script:

#!/bin/bash
# Description: Verify the updated `load` function call in another test context.
# Test: Search for the `load` function definition and usage. Expect: Consistent usage of the new parameter across different test cases.
rg --type typescript -A 5 'load'

Length of output: 67


Script:

#!/bin/bash
# Description: Verify the updated `load` function call in another test context.
# Test: Search for the `load` function definition and usage. Expect: Consistent usage of the new parameter across different test cases.
rg -A 5 'load'

Length of output: 223062

packages/utils/src/rundown-utils/rundownUtils.ts (1)

1-2: Updated imports and added new utility function.

The imports have been updated to include OntimeBlock and isOntimeBlock. This aligns with the new functionality introduced in the getRelevantBlock function.

apps/server/src/services/runtime-service/RuntimeService.ts (5)

104-106: Review of getRundown usage in checkTimerUpdate.

The method getRundown is correctly used here to fetch rundown data alongside playable events, which are then passed to the roll method of eventTimer. This ensures that the latest rundown data is used during the rolling process.


238-239: Review of getRundown usage in maybeUpdate.

Here, getRundown is used to fetch the latest rundown data for the reloadAll method. This is crucial for ensuring that the runtime state is updated with the most current data, especially when handling multiple events.


263-265: Review of getRundown usage in loadEvent.

The method getRundown is used to fetch the latest rundown data, which is essential for the load method in runtimeState. This ensures that the event loading process is based on the most current rundown information.


514-516: Review of getRundown usage in roll.

getRundown is used to fetch the latest rundown data, which is then used in the roll method of eventTimer. This is appropriate as it ensures that the event rolling process is based on the most current rundown data.


546-548: Review of getRundown usage in resume.

The method getRundown is used here to fetch the latest rundown data, which is crucial for the resume method in runtimeState. This ensures that the resume process is based on the most current rundown information.

packages/utils/src/rundown-utils/rundownUtils.ts Outdated Show resolved Hide resolved
Copy link
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.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1aaa07b and a6b6657.

Files selected for processing (1)
  • apps/client/src/features/cuesheet/Cuesheet.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (1)
  • apps/client/src/features/cuesheet/Cuesheet.tsx

Copy link
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.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a6b6657 and d9a139e.

Files selected for processing (1)
  • packages/utils/src/rundown-utils/rundownUtils.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • packages/utils/src/rundown-utils/rundownUtils.ts

Copy link
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.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d9a139e and 3cb3093.

Files ignored due to path filters (1)
  • packages/types/src/definitions/runtime/RuntimeStore.type.ts is excluded by none and included by none
Files selected for processing (6)
  • apps/client/src/common/stores/runtime.ts (1 hunks)
  • apps/server/src/app.ts (1 hunks)
  • apps/server/src/services/TimerService.ts (2 hunks)
  • apps/server/src/services/runtime-service/RuntimeService.ts (6 hunks)
  • apps/server/src/stores/tests/runtimeState.test.ts (4 hunks)
  • apps/server/src/stores/runtimeState.ts (13 hunks)
Files skipped from review as they are similar to previous changes (4)
  • apps/client/src/common/stores/runtime.ts
  • apps/server/src/services/TimerService.ts
  • apps/server/src/stores/tests/runtimeState.test.ts
  • apps/server/src/stores/runtimeState.ts
Additional comments not posted (6)
apps/server/src/app.ts (1)

182-185: Verify the correctness of the new currentBlock object.

Ensure that the currentBlock object is correctly initialized and used within the eventStore.init call. This change seems to be correctly adding the currentBlock object with block and startedAt properties initialized to null.

apps/server/src/services/runtime-service/RuntimeService.ts (5)

249-250: Verify the correctness of the getRundown function usage.

Ensure that the getRundown function is correctly used in the runtimeState.reloadAll call. This change seems to be correctly adding the rundown parameter to the reloadAll call.


274-276: Verify the correctness of the getRundown function usage.

Ensure that the getRundown function is correctly used in the runtimeState.load call. This change seems to be correctly adding the rundown parameter to the load call.


525-527: Verify the correctness of the getRundown function usage.

Ensure that the getRundown function is correctly used in the this.eventTimer.roll call. This change seems to be correctly adding the rundown parameter to the roll call.


557-559: Verify the correctness of the getRundown function usage.

Ensure that the getRundown function is correctly used in the runtimeState.resume call. This change seems to be correctly adding the rundown parameter to the resume call.


629-631: Verify the correctness of the currentBlock object usage.

Ensure that the currentBlock object is correctly used in the event store. This change seems to be correctly adding the currentBlock object to the event store.

Copy link
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.

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)
apps/server/src/services/timerUtils.ts (1)

147-148: Ensure consistent usage of the renamed parameter.

The parameter renaming from rundown to playableEvents is not consistent throughout the file. The following instances of rundown and rundownEvent need to be updated:

  • filteredRundown should be renamed to filteredPlayableEvents.
  • rundownEvent should be renamed to playableEvent.

Locations:

  • apps/server/src/services/timerUtils.ts

Please update these instances to ensure consistent usage of the renamed parameter.

Analysis chain

Ensure consistent usage of the renamed parameter.

The logic has been updated to use playableEvents instead of rundown. Verify that the parameter renaming is consistent throughout the function.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent usage of the renamed parameter `playableEvents`.

# Test: Search for the usage of `playableEvents` in the function. Expect: No occurrences of the old parameter name `rundown`.
rg --type-add 'ts' --type ts 'playableEvents' -A 5

Length of output: 108


Script:

#!/bin/bash
# Description: Verify consistent usage of the renamed parameter `playableEvents`.

# Test: Search for the usage of `playableEvents` and `rundown` in the relevant file.
rg 'playableEvents' apps/server/src/services/timerUtils.ts
rg 'rundown' apps/server/src/services/timerUtils.ts

Length of output: 1083

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 3cb3093 and 75fc00d.

Files ignored due to path filters (1)
  • packages/utils/index.ts is excluded by none and included by none
Files selected for processing (8)
  • apps/server/src/services/TimerService.ts (2 hunks)
  • apps/server/src/services/rundown-service/RundownService.ts (2 hunks)
  • apps/server/src/services/runtime-service/RuntimeService.ts (9 hunks)
  • apps/server/src/services/timerUtils.ts (2 hunks)
  • apps/server/src/stores/tests/runtimeState.test.ts (3 hunks)
  • apps/server/src/stores/runtimeState.ts (14 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.test.ts (2 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.ts (2 hunks)
Files skipped from review as they are similar to previous changes (4)
  • apps/server/src/services/TimerService.ts
  • apps/server/src/services/runtime-service/RuntimeService.ts
  • apps/server/src/stores/runtimeState.ts
  • packages/utils/src/rundown-utils/rundownUtils.ts
Additional comments not posted (8)
apps/server/src/stores/__tests__/runtimeState.test.ts (3)

101-101: LGTM!

The added assertion ensures that newState.currentBlock.block is correctly set to null after loading an event.


171-171: LGTM!

The added assertion ensures that newState.currentBlock.block is correctly set to null after loading an event in the runtime offset test.


203-203: LGTM!

The added assertion ensures that newState.currentBlock.block is correctly set to null after loading an event in the runtime offset test.

apps/server/src/services/rundown-service/RundownService.ts (2)

28-35: Improved readability!

The reformatting of the CompleteEntry type enhances code readability without altering functionality.


244-244: Verify the impact of the logic change.

The modification to pass only the affected variable to runtimeService.maybeUpdate suggests a shift in logic. Ensure that this change does not negatively impact the notification process to the timer service.

packages/utils/src/rundown-utils/rundownUtils.test.ts (2)

5-13: LGTM!

The addition of the getRelevantBlock function to the imports is necessary for the new tests.


268-322: Enhanced test coverage!

The new test suites for relevantBlock and filter event functions significantly improve the test coverage for the rundownUtils module.

apps/server/src/services/timerUtils.ts (1)

133-137: LGTM! Parameter renaming improves clarity.

The parameter rundown has been renamed to playableEvents, which enhances clarity by better reflecting the purpose of the argument.

@cpvalente
Copy link
Owner

Hi @alex-Arc , what is the status here?

Copy link
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.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 75fc00d and b0385e4.

Files selected for processing (3)
  • apps/server/src/services/runtime-service/RuntimeService.ts (9 hunks)
  • apps/server/src/stores/tests/runtimeState.test.ts (3 hunks)
  • apps/server/src/stores/runtimeState.ts (15 hunks)
Files skipped from review as they are similar to previous changes (2)
  • apps/server/src/services/runtime-service/RuntimeService.ts
  • apps/server/src/stores/tests/runtimeState.test.ts
Additional context used
GitHub Check: unit-test
apps/server/src/stores/runtimeState.ts

[failure] 198-198: src/stores/tests/runtimeState.test.ts > mutation on runtimeState > playback operations > normal playback cycle
TypeError: Cannot read properties of null (reading 'id')
❯ loadNow src/stores/runtimeState.ts:198:85
❯ Module.load src/stores/runtimeState.ts:171:3
❯ src/stores/tests/runtimeState.test.ts:96:7


[failure] 198-198: src/stores/tests/runtimeState.test.ts > mutation on runtimeState > playback operations > runtime offset
TypeError: Cannot read properties of null (reading 'id')
❯ loadNow src/stores/runtimeState.ts:198:85
❯ Module.load src/stores/runtimeState.ts:171:3
❯ src/stores/tests/runtimeState.test.ts:170:7

Additional comments not posted (8)
apps/server/src/stores/runtimeState.ts (8)

100-102: LGTM! The changes ensure correct state reset.

The updates to reset currentBlock and _prevCurrentBlock in the clear function are appropriate and necessary.


156-162: LGTM! The changes are consistent with the updated logic.

The modifications to the load function to include playableEvents and rundown parameters are appropriate and necessary for handling the new currentBlock logic.


235-235: LGTM! The changes are consistent with the updated logic.

The modifications to the loadNext function to use filterPlayable and handle the currentBlock logic are appropriate and necessary.


268-268: LGTM! The changes are consistent with the updated logic.

The modifications to the resume function to include playableEvents and rundown parameters are appropriate and necessary for handling the new currentBlock logic.


311-313: LGTM! The changes are consistent with the updated logic.

The modifications to the reloadAll function to include playableEvents and rundown parameters are appropriate and necessary for handling the new currentBlock logic.


338-341: LGTM! The changes are consistent with the updated logic.

The modifications to the start function to set currentBlock.startedAt if it is null are appropriate and necessary for handling the new currentBlock logic.


481-482: LGTM! The changes are consistent with the updated logic.

The modifications to the roll function to include playableEvents and rundown parameters are appropriate and necessary for handling the new currentBlock logic.


479-479: LGTM! The changes are consistent with the updated logic.

The modifications to the update function to handle the new currentBlock logic are appropriate and necessary.

apps/server/src/stores/runtimeState.ts Outdated Show resolved Hide resolved
Copy link
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.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b0385e4 and 61a7d10.

Files selected for processing (1)
  • apps/server/src/stores/runtimeState.ts (15 hunks)
Files skipped from review as they are similar to previous changes (1)
  • apps/server/src/stores/runtimeState.ts

@alex-Arc alex-Arc requested a review from cpvalente July 22, 2024 19:28
Copy link
Owner

@cpvalente cpvalente left a comment

Choose a reason for hiding this comment

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

I think this looks good, we need to find a way to show this in the interface (maybe the overview panel?) but lets do that in another PR

@cpvalente cpvalente merged commit 158ef05 into master Jul 23, 2024
3 checks passed
@cpvalente cpvalente deleted the relevantBlock branch July 23, 2024 10:22
@coderabbitai coderabbitai bot mentioned this pull request Sep 23, 2024
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.

Improve Cuesheet
2 participants