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.

Sign up for GitHub

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

Jump to bottom

SLVUU-117 add type guard for layoutJSON #133

Closed
wants to merge 3 commits into from
Closed

SLVUU-117 add type guard for layoutJSON #133

wants to merge 3 commits into from

Conversation

vferraro-scottlogic
Copy link

Changes:

  • add type guard for LayoutJSON
  • check layout is LayoutJSON before saving and show console error and error notification if it is not
  • check application layout is LayoutJSON before saving and show console error if it is not

@vferraro-scottlogic vferraro-scottlogic self-assigned this Dec 14, 2023
@vferraro-scottlogic vferraro-scottlogic added this to the Layout Management milestone Dec 14, 2023
@vferraro-scottlogic vferraro-scottlogic linked an issue Dec 14, 2023 that may be closed by this pull request
Empty JSON being saved to local storage #117
Open
cfisher-scottlogic
cfisher-scottlogic reviewed Dec 14, 2023
View reviewed changes
Copy link

@cfisher-scottlogic cfisher-scottlogic left a comment

Choose a reason for hiding this comment

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

Do we have any automated tests for this? How much do we gain by making one at this stage?

@vferraro-scottlogic vferraro-scottlogic changed the title SLVUU-117 add type guard for layoutJSON SLVUU-117 add type guard for layoutJSON-WIP Dec 18, 2023
@vferraro-scottlogic
Copy link
Author

Do we have any automated tests for this? How much do we gain by making one at this stage?

I am going to add some unit tests for useLayoutManager as there is none at the moment.
e2e tests-wise we have this ticket that will take care of that

@vferraro-scottlogic vferraro-scottlogic changed the title SLVUU-117 add type guard for layoutJSON-WIP SLVUU-117 add type guard for layoutJSON Jan 11, 2024
pling-scottlogic
pling-scottlogic reviewed Jan 11, 2024
View reviewed changes
Copy link

@pling-scottlogic pling-scottlogic left a comment

Choose a reason for hiding this comment

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

A few comments on the tests. I haven't looked too closely at the setup/mocking because it's not something I'm familiar with - I'm happy with that side of it if you're satisfied it's all working properly.

vuu-ui/packages/vuu-shell/test/layout-management/useLayoutManager.test.tsx Show resolved Hide resolved
vuu-ui/packages/vuu-shell/test/layout-management/useLayoutManager.test.tsx Outdated Show resolved Hide resolved
vuu-ui/packages/vuu-shell/test/layout-management/useLayoutManager.test.tsx Show resolved Hide resolved
pling-scottlogic
pling-scottlogic reviewed Jan 12, 2024
View reviewed changes
Copy link

@pling-scottlogic pling-scottlogic left a comment

Choose a reason for hiding this comment

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

I feel like there should be some more edge cases for the isLayoutJSON tests, but I can't think of any.

vuu-ui/packages/vuu-layout/test/typeOf.test.ts Show resolved Hide resolved
@pling-scottlogic
Copy link

PR merged into Finos

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cfisher-scottlogic cfisher-scottlogic cfisher-scottlogic left review comments

@pling-scottlogic pling-scottlogic pling-scottlogic approved these changes

@heswell heswell Awaiting requested review from heswell

Assignees

@vferraro-scottlogic vferraro-scottlogic

Labels
None yet
Projects
None yet
Milestone
Layout Management
Development

Successfully merging this pull request may close these issues.

Empty JSON being saved to local storage
3 participants
@vferraro-scottlogic @pling-scottlogic @cfisher-scottlogic