Open
Conversation
Contributor
WalkthroughRefactors onboarding tracker to return a discriminated union Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Resolver
participant Tracker as OnboardingTrackerService
participant FS as Filesystem/Override
rect rgba(200,220,255,0.5)
Client->>Resolver: GraphQL request (resolveOnboarding / mutation)
end
rect rgba(200,255,200,0.5)
Resolver->>Tracker: await getStateResult()
Tracker->>FS: read tracker file or read override
FS-->>Tracker: { kind: 'ok' | 'missing' | 'error', state? }
Tracker-->>Resolver: return OnboardingTrackerStateResult
end
alt kind === 'error'
Resolver->>Resolver: throw GraphQLError / propagate error
Resolver-->>Client: error response
else kind === 'ok' or 'missing'
Resolver->>Resolver: extract state and continue business logic
Resolver-->>Client: success response (onboarding data)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Ajit-Mehrotra
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1917 +/- ##
==========================================
+ Coverage 50.98% 51.12% +0.14%
==========================================
Files 1024 1024
Lines 70558 70628 +70
Branches 7714 7748 +34
==========================================
+ Hits 35974 36111 +137
+ Misses 34458 34393 -65
+ Partials 126 124 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/src/unraid-api/config/onboarding-tracker.service.ts (1)
57-109:⚠️ Potential issue | 🟠 MajorDon't leave
getState()as a non-erroring runtime read path.
getStateResult()only helps callers that opt in.api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts:32-61still builds its response fromgetState(), so those mutation paths can keep returning cached/default onboarding status when the tracker file can't be read. That means this PR fixes the customization query, but not all onboarding responses.Please migrate the remaining runtime readers to
getStateResult()or makegetState()internal/cached-only so tracker read failures are surfaced consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/config/onboarding-tracker.service.ts` around lines 57 - 109, The current getState() exposes a non-erroring runtime read path and must be made cached-only or replaced by an error-propagating API; change usages to call getStateResult() (which surfaces read errors) instead of getState() so tracker-read failures aren't silently ignored. Specifically, make getState() private or document it as cached-only, update public readers (isCompleted(), getCompletedAtVersion(), and any external callers such as the GraphQL mutation that currently uses getState()) to call getStateResult() and handle the returned OnboardingTrackerStateResult (extract state on kind==='ok' or propagate the error), and ensure readTrackerStateResult() remains the single source of truth for runtime reads. Use the function names getState(), getStateResult(), isCompleted(), getCompletedAtVersion(), getCurrentVersion(), and readTrackerStateResult() to locate and update all references.
🧹 Nitpick comments (3)
plugin/source/dynamix.unraid.net/install/doinst.sh (1)
40-45: Remove this duplicated symlink setup block.Line 40 through Line 45 duplicate the same
corepack/npm/npxlink recreation already done earlier, so this just re-runs identical operations. Keep only one block to avoid redundant install steps and future drift.Suggested diff
-( cd usr/local/bin ; rm -rf corepack ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/corepack/dist/corepack.js corepack ) -( cd usr/local/bin ; rm -rf npm ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npm-cli.js npm ) -( cd usr/local/bin ; rm -rf npx ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/source/dynamix.unraid.net/install/doinst.sh` around lines 40 - 45, Remove the duplicated symlink recreation block that repeats the corepack/npm/npx setup (the lines performing "( cd usr/local/bin ; rm -rf corepack )", "( cd usr/local/bin ; ln -sf ../lib/node_modules/corepack/dist/corepack.js corepack )", "( cd usr/local/bin ; rm -rf npm )", "( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npm-cli.js npm )", "( cd usr/local/bin ; rm -rf npx )", "( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx )"); keep the original single block earlier in the script and delete this repeated block so the symlinks for corepack, npm and npx are only created once.web/src/components/Onboarding/store/onboardingStatus.ts (1)
90-93: Verify partial-data GraphQL errors don’t over-block modal display.With
errorPolicy: 'all', Apollo can return bothresultanderror. Current logic blocks on any error unless a draft exists, which may hide the modal even when onboarding data is still present.Suggested adjustment (after verification)
- const canDisplayOnboardingModal = computed( - () => isVersionSupported.value && (hasResumableDraft.value || !hasOnboardingQueryError.value) - ); + const canDisplayOnboardingModal = computed( + () => + isVersionSupported.value && + (hasResumableDraft.value || Boolean(onboardingData.value) || !hasOnboardingQueryError.value) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/Onboarding/store/onboardingStatus.ts` around lines 90 - 93, The current blocking logic treats any Apollo error as fatal; change hasOnboardingQueryError to only consider an error blocking when there is no usable onboarding data returned. Replace the Boolean(onboardingError.value) check with one that also ensures the query result is empty (e.g., computed(() => Boolean(onboardingError.value) && !onboardingQueryResult.value?.data)), so canDisplayOnboardingModal uses errors only when there is truly no onboarding data; update references to onboardingError, onboardingQueryResult, hasOnboardingQueryError, and canDisplayOnboardingModal accordingly.api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts (1)
50-58: Avoid asserting the exact error text here.The behavior that matters is that tracker read failures reject. This assertion will fail on harmless message edits while the resolver behavior is still correct.
Based on learnings: "Test what the code does, not implementation details like exact error message wording - avoid writing tests that break when minor changes are made to error messages, log formats, or other non-essential details"♻️ Suggested tweak
- await expect(resolver.resolveOnboarding()).rejects.toThrow( - 'Onboarding tracker state is unavailable.' - ); + await expect(resolver.resolveOnboarding()).rejects.toThrow();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts` around lines 50 - 58, The test currently asserts the exact error message from resolver.resolveOnboarding; change it to only assert that the promise rejects (or rejects with an Error) rather than matching the exact text. Update the spec that mocks onboardingTracker.getStateResult (in this test using onboardingTracker.getStateResult and resolver.resolveOnboarding) to assert using await expect(resolver.resolveOnboarding()).rejects.toThrow() or .rejects.toBeInstanceOf(Error) instead of checking the specific message string so the test verifies failure behavior without depending on exact wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/__test__/store/onboardingStatus.test.ts`:
- Around line 74-82: Rename the test title to accurately reflect the asserted
behavior: change the it(...) description that currently says the modal is
blocked while loading to something like "blocks auto-show while the onboarding
query is still loading" so it matches the assertions on
canDisplayOnboardingModal and shouldShowOnboarding; update the test description
surrounding the useOnboardingStore invocation and the expectations for
canDisplayOnboardingModal and shouldShowOnboarding accordingly.
---
Outside diff comments:
In `@api/src/unraid-api/config/onboarding-tracker.service.ts`:
- Around line 57-109: The current getState() exposes a non-erroring runtime read
path and must be made cached-only or replaced by an error-propagating API;
change usages to call getStateResult() (which surfaces read errors) instead of
getState() so tracker-read failures aren't silently ignored. Specifically, make
getState() private or document it as cached-only, update public readers
(isCompleted(), getCompletedAtVersion(), and any external callers such as the
GraphQL mutation that currently uses getState()) to call getStateResult() and
handle the returned OnboardingTrackerStateResult (extract state on kind==='ok'
or propagate the error), and ensure readTrackerStateResult() remains the single
source of truth for runtime reads. Use the function names getState(),
getStateResult(), isCompleted(), getCompletedAtVersion(), getCurrentVersion(),
and readTrackerStateResult() to locate and update all references.
---
Nitpick comments:
In
`@api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts`:
- Around line 50-58: The test currently asserts the exact error message from
resolver.resolveOnboarding; change it to only assert that the promise rejects
(or rejects with an Error) rather than matching the exact text. Update the spec
that mocks onboardingTracker.getStateResult (in this test using
onboardingTracker.getStateResult and resolver.resolveOnboarding) to assert using
await expect(resolver.resolveOnboarding()).rejects.toThrow() or
.rejects.toBeInstanceOf(Error) instead of checking the specific message string
so the test verifies failure behavior without depending on exact wording.
In `@plugin/source/dynamix.unraid.net/install/doinst.sh`:
- Around line 40-45: Remove the duplicated symlink recreation block that repeats
the corepack/npm/npx setup (the lines performing "( cd usr/local/bin ; rm -rf
corepack )", "( cd usr/local/bin ; ln -sf
../lib/node_modules/corepack/dist/corepack.js corepack )", "( cd usr/local/bin ;
rm -rf npm )", "( cd usr/local/bin ; ln -sf
../lib/node_modules/npm/bin/npm-cli.js npm )", "( cd usr/local/bin ; rm -rf npx
)", "( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx )");
keep the original single block earlier in the script and delete this repeated
block so the symlinks for corepack, npm and npx are only created once.
In `@web/src/components/Onboarding/store/onboardingStatus.ts`:
- Around line 90-93: The current blocking logic treats any Apollo error as
fatal; change hasOnboardingQueryError to only consider an error blocking when
there is no usable onboarding data returned. Replace the
Boolean(onboardingError.value) check with one that also ensures the query result
is empty (e.g., computed(() => Boolean(onboardingError.value) &&
!onboardingQueryResult.value?.data)), so canDisplayOnboardingModal uses errors
only when there is truly no onboarding data; update references to
onboardingError, onboardingQueryResult, hasOnboardingQueryError, and
canDisplayOnboardingModal accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6e51ce32-b72a-4f08-9707-5c1451ccb2c2
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (9)
api/generated-schema.graphqlapi/src/unraid-api/config/onboarding-tracker.service.spec.tsapi/src/unraid-api/config/onboarding-tracker.service.tsapi/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.tsapi/src/unraid-api/graph/resolvers/customization/customization.resolver.tsplugin/source/dynamix.unraid.net/install/doinst.shweb/__test__/store/onboardingStatus.test.tsweb/src/components/Onboarding/store/onboardingDraft.tsweb/src/components/Onboarding/store/onboardingStatus.ts
- Purpose: tighten onboarding error handling so real API/tracker failures hide the modal, while missing tracker state and saved drafts can still resume the flow. - Before: onboarding tracker reads flattened missing and failed states together, and the modal gate did not preserve resumable drafts during restart windows. - Problem: summary-step API restarts could strand users mid-flow, while tracker failures could be misinterpreted as normal onboarding state. - Change: model tracker reads with explicit ok/missing/error results, surface true tracker failures as GraphQL errors, and allow the web modal to stay displayable when a resumable draft exists. - How: add tracker/result tests, update the customization resolver and generated GraphQL artifacts, and compute resumable draft visibility in the onboarding stores.
- Purpose: make the packaged plugin expose the expected package-manager entrypoints after install. - Before: the install script only recreated npm and npx links, leaving corepack unavailable from the installed environment. - Problem: package-manager workflows that rely on corepack could fail even though Node tooling was otherwise installed. - Change: create the corepack symlink during plugin install alongside the npm and npx links. - How: extend doinst.sh to link usr/local/bin/corepack to the bundled corepack entrypoint while preserving the existing npm and npx setup.
- Purpose: restore the web test suite after the onboarding branch was rebased onto newer mainline GraphQL changes. - Before: the generated GraphQL file contained two ServerStateDocument exports after conflict resolution. - Problem: Vite/esbuild failed during test transforms with a duplicate export error, causing a large slice of the web suite to fail before execution. - Change: remove the stale duplicate ServerStateDocument export and keep the version that matches the current serverState query shape. - How: clean up the generated GraphQL artifact so only the up-to-date ServerStateDocument remains.
- Purpose: resolve the outstanding CodeRabbit findings on PR #1917 after rebasing onto the latest mainline. - Before: the onboarding store over-blocked on partial Apollo errors, tracker read failures could still be flattened through mutation/runtime helper paths, one resolver spec asserted exact error text, and the plugin install script repeated the symlink block. - Problem: review comments highlighted behavior drift, brittle tests, and redundant install steps that could leave the PR confusing or incomplete. - Change: tighten the onboarding error gate for partial data, route runtime tracker readers through the error-propagating result path, relax the brittle spec assertion, rename the mismatched test, and collapse the duplicated install block. - How: update the API/web logic and focused tests for onboarding state handling, mutation responses, and tracker reads, then validate with targeted Vitest runs plus a shell syntax check for the install script.
Ajit-Mehrotra
force-pushed
the
codex/onboarding-error-fallback
branch
from
997272a to
98a07b3
Compare
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
web/__test__/store/onboardingStatus.test.ts (1)
162-172: Consider movingvi.mock()calls above the describe block for readability.While Vitest automatically hoists
vi.mock()calls, placing them after imports (before the describe block) is the conventional location and improves readability for developers scanning the file.♻️ Suggested reordering
import { useOnboardingDraftStore } from '~/components/Onboarding/store/onboardingDraft'; import { useOnboardingStore } from '~/components/Onboarding/store/onboardingStatus'; import { useServerStore } from '~/store/server'; +vi.mock('@vue/apollo-composable', () => ({ + useQuery: () => useQueryMock(), +})); + +vi.mock('~/components/Onboarding/store/onboardingDraft', () => ({ + useOnboardingDraftStore: vi.fn(), +})); + +vi.mock('~/store/server', () => ({ + useServerStore: vi.fn(), +})); + type OnboardingQueryResult = { // ... }; // ... rest of file ... -vi.mock('@vue/apollo-composable', () => ({ - useQuery: () => useQueryMock(), -})); - -vi.mock('~/components/Onboarding/store/onboardingDraft', () => ({ - useOnboardingDraftStore: vi.fn(), -})); - -vi.mock('~/store/server', () => ({ - useServerStore: vi.fn(), -}));As per coding guidelines: "Place all mock declarations at the top level in Pinia store tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/store/onboardingStatus.test.ts` around lines 162 - 172, Move the three vi.mock declarations for '@vue/apollo-composable', '~/components/Onboarding/store/onboardingDraft', and '~/store/server' out of the describe block and place them at the top-level (after the imports, before the describe) so they are easier to find; keep the mocked factories the same (useQueryMock for useQuery, vi.fn() for useOnboardingDraftStore and useServerStore) and ensure they remain hoisted by Vitest.api/src/unraid-api/config/onboarding-tracker.service.spec.ts (1)
171-186: Consider removing exact error message assertions.These tests assert the exact error message string
'permission denied'. Per coding guidelines, tests should use.rejects.toThrow()without arguments to avoid brittle tests that break when error messages change.♻️ Suggested refactor
- await expect(tracker.isCompleted()).rejects.toThrow('permission denied'); + await expect(tracker.isCompleted()).rejects.toThrow();- await expect(tracker.getCompletedAtVersion()).rejects.toThrow('permission denied'); + await expect(tracker.getCompletedAtVersion()).rejects.toThrow();As per coding guidelines: "Use
.rejects.toThrow()without arguments to test that functions throw errors. Don't test exact error message strings unless the message format is specifically what you're testing."Also applies to: 188-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/config/onboarding-tracker.service.spec.ts` around lines 171 - 186, The test asserts the exact error message 'permission denied' which is brittle; update the assertions in the OnboardingTrackerService spec so they only check that the promise rejects (use .rejects.toThrow() with no argument) for calls to tracker.isCompleted() after onApplicationBootstrap(), and similarly update the other occurrence noted (lines ~188-203) that uses mockReadFile to throw to avoid matching exact error strings; leave the mock behavior and error throw intact but remove the specific message argument from the .rejects.toThrow assertions.api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts (1)
50-57: Consider adding test coverage for the'missing'kind variant.The resolver treats both
'ok'and'missing'state kinds identically—both provide a state object that is processed normally, while only'error'throws. Currently, all successful tests use'ok', leaving no coverage for the'missing'variant. Adding a test case would document that a missing tracker file is a valid scenario (not an error) and ensure all three union type variants are exercised.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts` around lines 50 - 57, Add a new unit test that covers the onboardingTracker.getStateResult "missing" variant: mock vi.mocked(onboardingTracker.getStateResult) to resolve to { kind: 'missing' } and call resolver.resolveOnboarding(), asserting it does not throw and returns/handles the default state result the same way as the 'ok' path; place this alongside the existing tests for 'ok' and 'error' to ensure the resolver.resolveOnboarding behavior for the 'missing' kind is explicitly verified.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/src/unraid-api/config/onboarding-tracker.service.spec.ts`:
- Around line 171-186: The test asserts the exact error message 'permission
denied' which is brittle; update the assertions in the OnboardingTrackerService
spec so they only check that the promise rejects (use .rejects.toThrow() with no
argument) for calls to tracker.isCompleted() after onApplicationBootstrap(), and
similarly update the other occurrence noted (lines ~188-203) that uses
mockReadFile to throw to avoid matching exact error strings; leave the mock
behavior and error throw intact but remove the specific message argument from
the .rejects.toThrow assertions.
In
`@api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts`:
- Around line 50-57: Add a new unit test that covers the
onboardingTracker.getStateResult "missing" variant: mock
vi.mocked(onboardingTracker.getStateResult) to resolve to { kind: 'missing' }
and call resolver.resolveOnboarding(), asserting it does not throw and
returns/handles the default state result the same way as the 'ok' path; place
this alongside the existing tests for 'ok' and 'error' to ensure the
resolver.resolveOnboarding behavior for the 'missing' kind is explicitly
verified.
In `@web/__test__/store/onboardingStatus.test.ts`:
- Around line 162-172: Move the three vi.mock declarations for
'@vue/apollo-composable', '~/components/Onboarding/store/onboardingDraft', and
'~/store/server' out of the describe block and place them at the top-level
(after the imports, before the describe) so they are easier to find; keep the
mocked factories the same (useQueryMock for useQuery, vi.fn() for
useOnboardingDraftStore and useServerStore) and ensure they remain hoisted by
Vitest.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5db607d5-941a-40ea-9de7-b662abd53980
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (13)
api/generated-schema.graphqlapi/src/unraid-api/config/api-config.test.tsapi/src/unraid-api/config/onboarding-tracker.service.spec.tsapi/src/unraid-api/config/onboarding-tracker.service.tsapi/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.tsapi/src/unraid-api/graph/resolvers/customization/customization.resolver.tsapi/src/unraid-api/graph/resolvers/customization/onboarding.service.tsapi/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.tsapi/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.tsplugin/source/dynamix.unraid.net/install/doinst.shweb/__test__/store/onboardingStatus.test.tsweb/src/components/Onboarding/store/onboardingDraft.tsweb/src/components/Onboarding/store/onboardingStatus.ts
💤 Files with no reviewable changes (1)
- plugin/source/dynamix.unraid.net/install/doinst.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/components/Onboarding/store/onboardingStatus.ts
- web/src/components/Onboarding/store/onboardingDraft.ts
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
This PR hardens the onboarding flow so transient API problems do not incorrectly block or break the modal, while still hiding onboarding when we genuinely cannot trust the state.
It also includes the small plugin install-script cleanup that came along with this work and the generated GraphQL/schema updates that are currently part of the branch.
Behavior Changes
Onboarding tracker handling
onboarding-tracker.jsonfile is treated as a valid empty onboarding state rather than a read failure.getStateResult()path so customization and onboarding mutation responses behave consistently.Onboarding modal gating
Review follow-ups
corepack/npm/npxsymlink block from the plugindoinst.sh.Files / Areas Touched
Testing
API
pnpm exec vitest run src/unraid-api/config/onboarding-tracker.service.spec.ts src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts src/unraid-api/config/api-config.test.tsWeb
pnpm exec vitest run __test__/store/onboardingStatus.test.ts __test__/components/Onboarding/OnboardingModal.test.tspnpm testPlugin
sh -n plugin/source/dynamix.unraid.net/install/doinst.shSummary by CodeRabbit
Bug Fixes
Refactor