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

Vuu 81 definition as json node #104

Merged
merged 6 commits into from
Nov 17, 2023
Merged

Vuu 81 definition as json node #104

merged 6 commits into from
Nov 17, 2023

Conversation

vferraro-scottlogic
Copy link

@vferraro-scottlogic vferraro-scottlogic commented Nov 10, 2023 •
edited
Loading

Changes

  • use ObjectNode for definition instead of String/JsonNode

pling-scottlogic
pling-scottlogic requested changes Nov 13, 2023
View reviewed changes
layout-server/src/main/java/org/finos/vuu/layoutserver/dto/request/LayoutRequestDto.java Outdated Show resolved Hide resolved
layout-server/src/main/java/org/finos/vuu/layoutserver/utils/ObjectNodeConverter.java Outdated Show resolved Hide resolved
layout-server/src/test/java/org/finos/vuu/layoutserver/integration/LayoutIntegrationTest.java Outdated Show resolved Hide resolved
layout-server/src/test/java/org/finos/vuu/layoutserver/integration/LayoutIntegrationTest.java Outdated Show resolved Hide resolved
...ut-server/src/test/java/org/finos/vuu/layoutserver/service/ApplicationLayoutServiceTest.java Outdated Show resolved Hide resolved
...ut-server/src/test/java/org/finos/vuu/layoutserver/service/ApplicationLayoutServiceTest.java Outdated Show resolved Hide resolved
@vferraro-scottlogic vferraro-scottlogic force-pushed the VUU-81-definition-as-JsonNode branch from 427f2db to 8065a56 Compare November 15, 2023 11:13
@vferraro-scottlogic vferraro-scottlogic added this to the Layout Management milestone Nov 15, 2023
@vferraro-scottlogic vferraro-scottlogic linked an issue Nov 15, 2023 that may be closed by this pull request
Implement JsonNode in Layout DTOs #81
Open
@vferraro-scottlogic vferraro-scottlogic changed the base branch from main to VUU333-layout-server November 15, 2023 11:26
@vferraro-scottlogic vferraro-scottlogic force-pushed the VUU-81-definition-as-JsonNode branch from 8065a56 to 5911f5f Compare November 15, 2023 11:49
pling-scottlogic
pling-scottlogic requested changes Nov 15, 2023
View reviewed changes
layout-server/src/main/java/org/finos/vuu/layoutserver/utils/ObjectNodeConverter.java Show resolved Hide resolved
layout-server/src/main/java/org/finos/vuu/layoutserver/utils/ObjectNodeConverter.java Outdated Show resolved Hide resolved
layout-server/src/test/java/org/finos/vuu/layoutserver/integration/LayoutIntegrationTest.java Outdated Show resolved Hide resolved
layout-server/src/main/java/org/finos/vuu/layoutserver/dto/request/LayoutRequestDto.java Outdated Show resolved Hide resolved
layout-server/src/main/java/org/finos/vuu/layoutserver/dto/response/LayoutResponseDto.java Outdated Show resolved Hide resolved
layout-server/src/main/java/org/finos/vuu/layoutserver/dto/request/LayoutRequestDto.java Show resolved Hide resolved
vferraro-scottlogic
vferraro-scottlogic commented Nov 15, 2023
View reviewed changes
...ut-server/src/test/java/org/finos/vuu/layoutserver/service/ApplicationLayoutServiceTest.java

service.persistApplicationLayout(user, definition);

verify(mockRepo, times(1))
.save(new ApplicationLayout(user, definition));
}

@Test
public void createApplicationLayout_invalidDefinition_throwsJsonException() {
Copy link
Author

Choose a reason for hiding this comment

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

this test case is not relevant anymore as definition is now an ObjectNode rather than a string

pling-scottlogic
pling-scottlogic approved these changes Nov 16, 2023
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.

Good stuff! Just a very minor thing on wording, but that won't stop me from approving.

layout-server/src/test/java/org/finos/vuu/layoutserver/integration/LayoutIntegrationTest.java Outdated
@@ -32,14 +39,16 @@
@ActiveProfiles("test")
public class LayoutIntegrationTest {

private static final String DEFAULT_LAYOUT_DEFINITION = "Default layout definition";
private static final String DEFAULT_LAYOUT_DEFINITION_STRING = "{\"id\":\"main-tabs\"}";
private static final String DEFAULT_UPDATED_LAYOUT_DEFINITION_STRING = "{\"id\":\"updated-main-tabs\"}";
Copy link

Choose a reason for hiding this comment

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

Minor semantic thing that's not going to stop me from approving: I would remove the word "default" and just call this UPDATED_LAYOUT_DEFINITION_STRING.

@vferraro-scottlogic vferraro-scottlogic merged commit 7fdeaa7 into VUU333-layout-server Nov 17, 2023
7 checks passed
@pling-scottlogic pling-scottlogic mentioned this pull request Nov 17, 2023
Merged
@cfisher-scottlogic cfisher-scottlogic mentioned this pull request Nov 28, 2023
Open
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 approved these changes

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

@heswell heswell Awaiting requested review from heswell

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Layout Management
Development

Successfully merging this pull request may close these issues.

Implement JsonNode in Layout DTOs
3 participants
@vferraro-scottlogic @pling-scottlogic @cfisher-scottlogic