JNG-6169 transform table card representation #293
Conversation
noherczeg
requested review from
robertcsakany,
bencegelei,
gaborflorian and
Madmat8
WalkthroughThis pull request introduces multiple changes to the UI transformation and testing layers. It adds new import statements and operations (especially for card declarations), enhances ETL transformation rules with new UI component generation, revises several guard conditions and rule annotations (transitioning from @greedy to @lazy), and implements inline editing capabilities. In addition, test files have been updated to reflect a shift from table-based to row-based representations, and a dependency version in the pom.xml file has been updated. Changes
Sequence Diagram(s)sequenceDiagram
participant SM as JSL Model
participant TE as Transformation Engine
participant UI as UI Components
SM->>TE: Import and provide model declarations
TE->>TE: Apply new operations and transformation rules
TE->>TE: Evaluate guards, adjust annotations (@greedy to @lazy), and set inline editing
TE->>UI: Generate updated UI elements (cards, tables → rows, actions, pages)
Poem
✨ Finishing Touches
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (4)
judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/viewTableDeclaration.eol (1)
14-24: 💡 Verification agent🧩 Analysis chain
Verify if the commented-out link operations are still needed.
The removal of
getAllLinks()andgetDetailLink()operations suggests a shift away from row-based links. Please confirm if this functionality has been moved elsewhere or is no longer needed.
🏁 Script executed:
#!/bin/bash # Description: Check if these operations are used elsewhere in the codebase # or if they have been replaced with card-based alternatives # Search for getAllLinks usage echo "Searching for getAllLinks usage..." rg "getAllLinks\(\)" -A 2 # Search for getDetailLink usage echo "Searching for getDetailLink usage..." rg "getDetailLink\(\)" -A 2 # Search for potential card-based alternatives echo "Searching for potential card-based alternatives..." rg "getCard.*Link" -A 2Length of output: 3621
Attention: Inconsistent Removal of Link Operations
Our search reveals that while the
getAllLinks()operation has an alternative implementation inviewDeclaration.eol, thegetDetailLink()operation, which is still referenced in transformation files (e.g., in bothviewLinkDeclarationFormPage.etlandviewTableDeclarationFormPage.etl), exists only in the commented-out block inviewTableDeclaration.eol. This indicates a potential issue where transformation files may be expectinggetDetailLink()even though it’s been removed. Please clarify if the removal of these operations was intentional and if the transformation logic should be updated, or ifgetDetailLink()needs to be reintroduced to support the existing references.judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTableDeclarationViewPage.etl (3)
131-145: 🛠️ Refactor suggestionConsider adding error handling for unknown reference types.
The conditional logic for filter actions should handle cases where the reference type is neither UIRowDeclaration nor UICardDeclaration.
Apply this diff to add error handling:
if (s.referenceType.isTypeOf(JSL!UIRowDeclaration)) { t.actionDefinition = s.equivalent("ViewTableDeclarationFilterActionDefinition"); } else if (s.referenceType.isTypeOf(JSL!UICardDeclaration)) { t.actionDefinition = s.equivalent("InlineViewCardsFilterButtonActionDefinition"); +} else { + throw "Unsupported reference type: " + s.referenceType.type().name; }
147-163: 🛠️ Refactor suggestionAdd similar error handling for refresh actions.
The refresh action rule has the same potential issue with unknown reference types.
Apply similar error handling as suggested for the filter actions.
131-247: 🛠️ Refactor suggestionAdd error handling for unknown reference types.
The conditional logic should handle unknown types gracefully:
if (s.referenceType.isTypeOf(JSL!UIRowDeclaration)) { t.actionDefinition = s.equivalent("ViewTableDeclarationFilterActionDefinition"); } else if (s.referenceType.isTypeOf(JSL!UICardDeclaration)) { t.actionDefinition = s.equivalent("InlineViewCardsFilterButtonActionDefinition"); +} else { + var msg = "Unsupported reference type: " + s.referenceType.type().name; + log.error(msg); + throw new IllegalArgumentException(msg); }
🧹 Nitpick comments (29)
judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTableDeclarationFormPage.etl (1)
43-64: Optimization: Switching from greedy to lazy evaluation.The change from
@greedyto@lazyfor both action rules is a good optimization. This ensures that these transformations are only executed when needed, potentially improving performance by reducing unnecessary evaluations.This change is part of a broader pattern across files, suggesting a systematic optimization of the transformation pipeline.
judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/rowDeclaration.etl (5)
176-176: Consider making inline editing configurable.The
isInlineEditableproperty is unconditionally set totruefor all tables. Consider making this configurable based on the JSL declaration to allow more granular control over which tables support inline editing.- t.isInlineEditable = true; + t.isInlineEditable = s.hasInlineEditingEnabled();
194-195: Address the TODO comment about selector row per page.The hardcoded value of 10 for
selectorRowsPerPageshould be configurable.Would you like me to help implement a configurable pagination system that allows customizing the number of rows per page?
208-208: Address the TODO comment about adding buttons.The comment suggests that additional buttons might need to be added to the table action button group.
Would you like me to help identify and implement any missing button actions for the table?
228-228: Address the TODO comment about adding buttons.Similar to the table action button group, this comment suggests that additional buttons might need to be added to the row action button group.
Would you like me to help identify and implement any missing button actions for the rows?
441-441: Consider using distinct icons for different actions.The "link-off" icon is used for both "Clear" and "BulkRemove" actions, while "attachment-plus" is used for "OpenAddSelector". Using the same icon for different actions might confuse users.
Consider using more specific icons:
- For "Clear": Use "clear-all" or "delete-sweep"
- For "BulkRemove": Keep "link-off"
- For "OpenAddSelector": Keep "attachment-plus"
Also applies to: 478-478, 516-516
judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiOperationsTest.java (1)
326-350: Consider adding negative test cases.While the positive test cases are well covered, consider adding negative test cases to verify error handling for scenarios like:
- Invalid row configurations
- Missing required columns
- Duplicate column names
judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/menuTableDeclarationTablePage.etl (1)
1-47: Consider architectural implications of the transformation rules.The
AccessTablePageDefinitionrule is responsible for generating UI page definitions with various actions. With the new guard condition restricting it to row declarations, ensure that:
- Card-based views have equivalent functionality
- The transformation maintains feature parity
Consider:
- Documenting the relationship between row and card declarations
- Adding validation to ensure all required actions are available in both views
- Creating integration tests to verify the transformation behavior
judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiRowOperationsTest.java (1)
224-225: Remove duplicate assertion.The assertion
isOpenOperationInputFormAction()is duplicated on lines 224 and 225.- assertTrue(accessTableMyAction3.isOpenOperationInputFormAction()); - assertTrue(accessTableMyAction3.isOpenOperationInputFormAction()); + assertTrue(accessTableMyAction3.isOpenOperationInputFormAction());judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTableDeclaration.etl (4)
15-15: Consider making inline editing optional or configurable.
While always settingt.isInlineEditable = truemight satisfy most use cases, making it conditional or configurable could provide more flexibility for scenarios where inline editing isn't desired.
63-77: Follow through on the TODO inInlineViewCardsButtonGroup.
A TODO comment indicates for additional buttons to be added. Completing this will ensure a fully functional button group.Do you need help adding the remaining buttons or clarifying their functionality?
80-92: Consider internationalizing hard-coded labels.
Labels like"Filter"are user-facing. If you need multi-language support, consider pulling label text from a localization resource.
152-165: Address the TODO for row buttons.
A TODO indicates more row-level actions or buttons might be added. Resolving this ensures feature completeness.Would you like assistance drafting the additional row-based actions?
judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/viewTableDeclaration.eol (1)
31-54: Add documentation for the getExposedVisualElements operation.While the implementation is solid, adding documentation would help explain:
- The purpose of the operation
- The expected content of the input Set
- What constitutes an "exposed" visual element
- The significance of circular reference prevention
Add this documentation above the operation:
+/** + * Collects all exposed visual elements from this table declaration and its related components. + * + * @param ves Set to store and detect already processed visual elements (prevents circular references) + * @return Set of all exposed visual elements + */ operation JSL!UIViewTableDeclaration getExposedVisualElements(ves: Set): Set {judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/menuTableDeclarationCardsPage.etl (1)
41-50: Unify repeated logic if needed
AccessCardsTableRefreshActionfollows a nearly identical pattern to other action rules. If you anticipate more actions of similar structure, consider refactoring into a shared utility or template to avoid duplication.judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/cardDeclaration.etl (7)
1-28: Fallback for missingApplication
This rule effectively creates aPageContainer. Consider adding a safety check forrootMenu.equivalent("Application")to handle unexpected scenarios gracefully.
60-71: Button style considerations
Using"text"style for a Back button is a common pattern. If styling guidelines change in the future, centralizing style definitions may help maintain consistency.
74-87: Flex container
Using a verticalFlexis good for stacking elements. If further spacing or margin is needed, consider adding relevant styling or parameters.
90-123: Robust table configuration
Inline editing, filters, and columns are well-managed here. Just confirm that__posusage won’t conflict with other transformations that may set positions differently.
126-139: Track the TODO
A placeholder comment states// TODO add buttons. Ensure to create a follow-up issue or backlog item so these actions are not overlooked.
157-168: Styling consistency
The Filter button is set to"text". Make sure all similarly important text-based actions share a consistent look and feel.
193-204: Refresh action prominence
The refresh button is"text"style, which can be subtle. Consider a more prominent style if refreshes are critical for users.judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/cardDeclaration.eol (2)
14-22: Exposed visual elements
This logic is sound. If nested card structures emerge later, verify that recursion or deeper traversal is handled.
24-31: ActionDeclarations retrieval
Gathering allUIActionDeclarationmembers is adequate for current needs. If actions become nested or inherited, consider expanding this approach.judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JSLModel2UiListItemTest.java (3)
37-102: Consider enhancing the model string creation.The model string creation method could benefit from some improvements:
- The method lacks documentation explaining the model structure
- The indentation in the model string makes it harder to read
Consider applying this diff to improve readability and maintainability:
private static String createModelString(String name) { + // Creates a test model with: + // - Two entities (Entity1, Entity2) with relations + // - Two transfers (Transfer1, Transfer2) with update events + // - Two views (View1, View2) with widgets and tables + // - Two cards (Card1, Card2) with widgets + // - One actor (A) with access to transfers + // - One menu (M) with table declarations return """ - model %s; + model %s; - import judo::types; + import judo::types;
37-102: Enhance test coverage for card transformations.While the model structure is comprehensive, consider adding test cases for:
- Card validation scenarios
- Error cases for invalid card configurations
- Edge cases with empty or null field values
111-220: Improve test maintainability.Consider extracting frequently used test values into constants to improve maintainability:
+ private static final String EXPECTED_DASHBOARD_PAGE = "A::Cards::M::DashboardPage"; + private static final String EXPECTED_ACCESS_CARDS_PAGE = "A::Cards::M::v1s::AccessCardsPage"; + private static final String EXPECTED_ACCESS_TABLE_VIEW_PAGE = "A::Cards::M::v1s::AccessTableViewPage"; + private static final String EXPECTED_VIEW_PAGE = "A::Cards::View1::level::relatedCollection::ViewPage"; assertEquals(List.of( - "A::Cards::M::DashboardPage", - "A::Cards::M::v1s::AccessCardsPage", - "A::Cards::M::v1s::AccessTableViewPage", - "A::Cards::View1::level::relatedCollection::ViewPage" + EXPECTED_DASHBOARD_PAGE, + EXPECTED_ACCESS_CARDS_PAGE, + EXPECTED_ACCESS_TABLE_VIEW_PAGE, + EXPECTED_VIEW_PAGE ), pages.stream().map(NamedElement::getFQName).sorted().toList());judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTableDeclarationViewPage.etl (2)
178-196: Consider refactoring duplicate reference type checks.The same reference type checking pattern is repeated across multiple rules.
Consider extracting the reference type check into a helper operation:
+operation JSL!UIViewTableDeclaration getActionDefinitionByType(rowDef : String, cardDef : String) : String { + if (self.referenceType.isTypeOf(JSL!UIRowDeclaration)) { + return rowDef; + } else if (self.referenceType.isTypeOf(JSL!UICardDeclaration)) { + return cardDef; + } else { + throw "Unsupported reference type: " + self.referenceType.type().name; + } +}
1-130: Add documentation for transformation rules.The rules are well-structured, but adding documentation would improve maintainability:
+/** + * Transforms a JSL view table declaration into a UI page definition. + * @param s The source JSL view table declaration + * @param t The target UI page definition + */ rule ViewTableViewPageDefinition transform s: JSL!UIViewTableDeclaration to t: UI!ui::PageDefinition {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/_importUI.eol(1 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/cardDeclaration.eol(1 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/menuTableDeclaration.eol(1 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/viewTableDeclaration.eol(1 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/jslToUi.etl(1 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/structure/transferActionDeclaration.etl(0 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/cardDeclaration.etl(1 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/menuTableDeclarationCardsPage.etl(1 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/menuTableDeclarationTablePage.etl(1 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/rowColumnDeclaration.etl(0 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/rowDeclaration.etl(1 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewLinkDeclaration.etl(0 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewLinkDeclarationFormPage.etl(0 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewLinkDeclarationViewPage.etl(0 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTableDeclaration.etl(2 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTableDeclarationFormPage.etl(1 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTableDeclarationViewPage.etl(3 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTabsDeclaration.etl(0 hunks)judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewWidgetDeclaration.etl(1 hunks)judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JSLModel2UiListItemTest.java(1 hunks)judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiActionGroupsTest.java(3 hunks)judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiApplicationTest.java(4 hunks)judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiCRUDTest.java(8 hunks)judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiCarTest.java(3 hunks)judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiDataTest.java(3 hunks)judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiNavigationTest.java(3 hunks)judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiOperationsTest.java(6 hunks)judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiRowOperationsTest.java(8 hunks)judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiWidgetsTest.java(6 hunks)pom.xml(1 hunks)
💤 Files with no reviewable changes (6)
- judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewLinkDeclaration.etl
- judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTabsDeclaration.etl
- judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewLinkDeclarationFormPage.etl
- judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/structure/transferActionDeclaration.etl
- judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/rowColumnDeclaration.etl
- judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewLinkDeclarationViewPage.etl
✅ Files skipped from review due to trivial changes (3)
- judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/_importUI.eol
- judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/menuTableDeclaration.eol
- judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/jslToUi.etl
🔇 Additional comments (84)
judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTableDeclarationFormPage.etl (1)
5-5: LGTM! Guard condition aligns with table-to-card transformation.The additional type check
s.referenceType.isTypeOf(JSL!UIRowDeclaration)ensures that the transformation only applies to row-based declarations, which aligns with the PR's objective of transforming table representations.judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiDataTest.java (4)
289-292: LGTM! Consistent renaming of table definitions to row definitions.The renaming from
tabletorowforTransfer1RowandTransfer2Rowis consistent with the PR's objective of transforming table card representation.Also applies to: 294-296
304-305: LGTM! Updated menu references to use row-based naming.The menu item references have been correctly updated to use the new row-based naming convention while maintaining the same functionality.
417-417: LGTM! Consistent updates to view table references.All table references in the view have been consistently updated to use the row-based naming convention:
UnmappedRelatedRowMappedRelatedRowAlso applies to: 420-420, 424-424, 427-427, 430-430, 433-433, 436-436, 439-439
446-448: LGTM! Consistent definition of row-based components.The definitions of
UnmappedRelatedRowandMappedRelatedRowmaintain the same structure and functionality while adopting the new row-based naming convention.Also applies to: 454-456
judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiCarTest.java (4)
102-105: LGTM! Clean row declaration with proper column mappings.The transition from table to row representation maintains the correct field mappings while aligning with the PR objectives.
108-108: LGTM! Menu declaration properly updated for row-based representation.The array notation
CarRow[]correctly implements the new row-based approach while maintaining the form and view references.
143-143: LGTM! Page container name updated consistently.The page container name has been properly updated to reflect the new row-based structure while maintaining the correct path format.
157-157: LGTM! Table name updated consistently.The table name assertion has been properly updated to reflect the new row-based structure.
judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiOperationsTest.java (3)
63-63: LGTM! Consistent renaming from table to row-based representation.The changes align with the PR's objective of transforming table card representation to a row-based approach. The modifications maintain functionality while improving the clarity of the data structure representation.
Also applies to: 100-100, 108-108, 117-117
167-168: LGTM! Test assertions updated to match row-based naming.The assertions have been correctly updated to verify the new row-based page containers, maintaining test coverage for the refactored components.
326-327: LGTM! Selector table assertions updated consistently.All FQName assertions and variable names have been properly updated to use the new row-based naming convention, maintaining test consistency.
Also applies to: 329-329, 339-344, 348-349
judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiActionGroupsTest.java (4)
99-99: LGTM! Array notation correctly matches the relation definition.The change from
table Table2totable Table2[]properly aligns with theTransfer2[]relation defined inTransfer1.
109-111: LGTM! Transformation to row-based representation is correct.The change from
tabletorowaligns with the PR objective of transforming table card representation while maintaining the same parameter structure and column definition.
113-115: LGTM! Consistent transformation to row-based representation.The change from
tabletorowmaintains consistency with the Table1 transformation and aligns with the PR objective.
122-122: LGTM! Array notation correctly matches the access definition.The change from
table Table1totable Table1[]properly aligns with theTransfer1[]access defined in actor A.judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/menuTableDeclarationTablePage.etl (1)
4-4: Verify the impact of the additional type check.The guard condition now ensures that only
UIRowDeclarationtypes are processed. This change appears to be part of the table-to-card transformation initiative.Please verify:
- All existing table declarations are properly handled
- No unintended side effects on existing functionality
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for all UIMenuTableDeclaration usages to ensure they're properly handled ast-grep --pattern 'transform s: JSL!UIMenuTableDeclaration' # Search for UIRowDeclaration references to understand the context rg -A 5 'UIRowDeclaration'Length of output: 44743
Guard Condition Consistency Confirmed
The additional type check (
s.referenceType.isTypeOf(JSL!UIRowDeclaration)) is in line with similar guard conditions in related transformation files (e.g., in both viewTableDeclarationFormPage.etl and viewTableDeclaration.etl). This change consistently restricts processing to row declarations, ensuring that only the intended elements are transformed and that alternate types (like card declarations) are handled in their respective transformation rules. There’s no evidence of removed or replaced code that would adversely affect existing table declarations.
- The guard condition is consistent with analogous rules in the codebase.
- No unintended side effects were identified upon reviewing related transformations.
judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiRowOperationsTest.java (2)
171-172: LGTM! Test assertions correctly updated.The assertions have been properly updated to reflect the transition from table-based to row-based naming while maintaining the same validation coverage.
Also applies to: 185-190, 200-204, 219-223
76-83: LGTM! Consistent renaming from table to row.The changes correctly implement the transition from table-based to row-based representation while maintaining the same structure and functionality.
Let's verify that all table references have been updated to row references:
Also applies to: 117-122, 129-132, 139-139
❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Description: Verify no remaining table references in model string # Test: Search for "table" keyword in model declarations. Expect: Only table keyword in view declarations. rg -U 'table [A-Za-z0-9]+\(' 'judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiRowOperationsTest.java'Length of output: 149
I'll run a follow-up script to verify that the file now contains the expected "row" declarations (for example, for
Row1andRow3) and that no lingering "table" declarations exist in the model strings.Please run the following shell script:
#!/bin/bash # Description: Verify that the updated model definitions now use "row" declarations # Check for "table" declarations to ensure they have been replaced echo "Searching for 'table' declarations:" rg -U 'table [A-Za-z0-9]+\(' judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiRowOperationsTest.java echo "Searching for 'row Row1' declaration:" rg -U 'row Row1\(' judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiRowOperationsTest.java echo "Searching for 'row Row3' declaration:" rg -U 'row Row3\(' judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiRowOperationsTest.javaOnce we confirm that no "table" references appear and that the expected "row" declarations for
Row1andRow3are present, we can conclude that the modifications are correctly applied.Please proceed with running the script and share the output if further investigation is needed.
judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewWidgetDeclaration.etl (1)
5-5: LGTM! Verify card declaration transformation rules.The guard condition modification correctly prevents widget transformations within card declarations, aligning with the PR objective of transforming table card representation.
Let's verify that card declarations are handled by separate transformation rules:
❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Description: Search for card declaration transformation rules # Search for card declaration rules echo "Searching for card declaration transformation rules..." rg -l "UICardDeclaration" --type etl # Search for specific card transformation patterns echo "Searching for specific card transformation patterns..." ast-grep --pattern 'rule $_ transform s: JSL!UICardDeclaration to t: UI!ui::$_'Length of output: 384
Guard Condition Change Approved – Pending Verification of Card Transformation Rules
The updated guard condition correctly excludes widget transformations for visual elements contained within card declarations, which aligns with the goal to transform table card representations differently. However, our automated search (adjusted to remove the unrecognized ETL file type flag) did not reveal any explicit transformation rules handling
JSL!UICardDeclaration. Please verify manually that card declarations are indeed processed by a dedicated transformation rule elsewhere in the repository.
- The guard condition in
AbstractViewWidgetDeclarationis correct.- No distinct transformation rule for handling card (i.e.
JSL!UICardDeclaration) declarations was automatically detected.To further investigate, you can run a search in all files for references to
JSL!UICardDeclaration:#!/bin/bash echo "Searching for any transformation rules involving 'JSL!UICardDeclaration' in all files..." rg -l "JSL!UICardDeclaration" || echo "No matches found" echo "Searching for transformation rules with pattern 'rule .* transform .*JSL!UICardDeclaration'..." rg -n "rule .* transform .*JSL!UICardDeclaration" || echo "No matches found"Review the results to confirm that card declarations are handled appropriately.
judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTableDeclaration.etl (10)
32-61: Validate container usage and consider dynamic rows-per-page.
The code referencess.eContainer.uiContainer()and setst.selectorRowsPerPage = 10. Ensures.eContaineris never null, and consider makingselectorRowsPerPagedynamic if the desired page size changes.
94-103: Looks good.
Icon creation is straightforward and consistent with the rest of the code.
105-114: Verify action definition dependencies.
Ensures.transferRelationand itstarget.referenceTypeare always defined before creating aFilterActionDefinition.
116-128: Implementation is consistent.
Providing a refresh button that leverages an action definition matches the overall approach elsewhere.
130-139: Implementation matches the established icon pattern.
No concerns here; the naming convention is consistent.
141-150: Refresh action definition looks correct.
This rule appears consistent with prior patterns for action definitions.
168-179: Consistent approach for an “open page” button.
Naming and references align with the surrounding patterns.
181-190: InlineViewCardsOpenPageButtonIcon is well-defined.
No issues; the icon usage is clear.
192-201: Open page action behavior appears correct.
This action definition leverages the correct targetType reference and naming structure.
207-207: Ensure the new guard condition is always valid.
Double-check thats.referenceTypeis defined and of the correct type to prevent runtime errors.judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/viewTableDeclaration.eol (1)
36-39: LGTM! Improved variable naming.The renaming from
rowDeclarationtolistItemDeclarationbetter reflects the generic nature of the reference type and aligns with the transformation from table-specific to card-based representation.judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiCRUDTest.java (15)
99-99: Shift to a row-based DSL is coherent.
The introduction ofrow UserRow(UserTransfer u)aligns with the newly adopted row-centric pattern.
106-107: Renaming link and table references to row-based notation.
These changes correctly reflect the transformation from table-based elements to row-based entities in the DSL.
111-111: Consistent row definition.
row RelatedRow(RelatedTransfer r)is a straightforward renaming that aligns with the new row structure.
120-122: Row-based DSL usage in linked elements.
Shifting from table to row references forreadOnlyJumper,myJumper, andmyJumpersmaintains consistency throughout the code.
136-136: Row creation for Jumper.
row JumperRow(JumperTransfer j)cleanly supports the row-based approach.
153-153: Consistent array-based user row definition.
table UserRow[] usersclearly replaces the earlier table notion with a row-array structure.
205-205: Updated references to row-based entities in summary array.
These string constants now correctly reflectJumperRow::Table,RelatedRow::Table, andUserRow::Tablein alignment with the row approach.Also applies to: 209-209, 211-211
242-243: Refined naming references for row-based tables.
Ensuring that each entity (JumperRow,RelatedRow,UserRow) is tracked as a “Table” object in the UI model is consistent with the DSL refactor.Also applies to: 245-245
770-770: Maintaining the Table object for add-selector usage.
DefiningmyJumpersAddSelectoras aTablecontainer forJumperRowentries is valid and consistent with the row DSL.
777-782: Button group references for JumperRow add-selector.
These changes ensure that the row-based DSL actions (BulkRemove,Clear, etc.) map correctly toJumperRow.
785-785: Consistent filter button association.
myJumpersAddSelectorFilterproperly referencesJumperRow::Filterin line with the row usage.
811-811: Set-selector container for row items.
myJumperSetSelectorremains aTableobject, which aligns with the approach of hosting multipleJumperRowentries.
817-822: Refined row-based table button group references.
These strings reference standard row-level actions (BulkRemove,Clear,Filter,OpenAddSelector, etc.) forJumperRowand match the DSL shift.
825-825: Row-based filter button definition.
myJumperSetSelectorFilterappropriately aligns withJumperRow::Filterin the row DSL.
830-830: Refresh action aligned with row DSL.
myJumperSetSelectorRefreshcorrectly supports the row-based refresh action forJumperRow.judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/menuTableDeclarationCardsPage.etl (4)
1-28: Ensure presence ofactorAccessbefore usage
It's not explicitly verified ifs.actorAccessis always defined. If it isn't, referencings.actorAccess.targetmight cause runtime issues. Consider adding a check or guard condition.
30-39: Looks good at a glance
The transformation logic here is straightforward, setting ID, name, andactionDefinition. No issues found in this rule.
52-61: Verify filter condition usage
Ensures.referenceTypesupportsequivalent("CardTableFilterActionDefinition")and that filters are only added when relevant to the domain logic.
63-75: Confirm target page definition
t.targetPageDefinitionis set tos.equivalent("AccessTableViewPageDefinition"). Double-check that "AccessTableViewPageDefinition" is declared to avoid null references or runtime issues.judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/cardDeclaration.etl (14)
29-40: Future expansion
The code sets up aButtonGroupwith a single Back button. The TODO note indicates more actions may come. The current approach looks modular enough to accommodate them.
42-49: No issues with the icon
The chosen icon is"arrow-left". Everything appears correct, assuming the icon library supports that name.
52-58: Naming consistency
CardPageContainerBackActionDefinitionaligns well with the action's purpose. No further suggestions needed.
142-154: Row-level button group
Attaching row actions is a good design approach. Double-check behavior for edge cases (e.g., empty rows).
171-179: Filter icon reference
The"filter"icon is used. Confirm that the icon resource name matches your UI framework’s library.
182-190: Aligned filtering
The filter definition setst.targetTypeconsistent with the table’sClassType. This should enable coherent filtering logic.
207-215: Check icon name
The"refresh"icon is referenced. Make sure it is rectly spelled and present in your icon set.
218-226: Proper invocation of refresh
TheRefreshActionDefinitionshould be triggered on appropriate events. Consider ensuring it syncs with the UI state to avoid incomplete refresh states.
229-240: Contained style
Using a"contained"button style for the “View” action emphasizes its importance. This aligns well with typical UI patterns.
243-251: Visibility icon
“visibility” is a clear choice for a view action. This is intuitive for end users.
254-262: Target definition
CardOpenPageActionDefinitionreferences a potential page resource. Verify that the resource is properly defined to prevent null references during runtime.
265-279: Column definition
Configuring label, format, and optional icon is well-handled. No issues noted here.
282-290: Dynamic icon name
Pulling the icon name froms.getIconModifier().value.valuecould fail if the DSL provides unexpected input. Consider verifying or sanitizing the value.
293-301: Column filters
Each column's filter is created consistently. This helps achieve a uniform user experience.judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/cardDeclaration.eol (1)
6-12: Safe fallback
Here, thegetFqNameoperation checks ifeContaineris defined. The fallback toself.nameis a good defensive approach.judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JSLModel2UiListItemTest.java (4)
1-19: LGTM! Well-structured test class setup.The class is properly organized with necessary imports and extends AbstractTest appropriately.
111-178: Verify test coverage for edge cases.The test verifies basic functionality but could be enhanced to cover edge cases:
- Empty collections
- Null values in fields
- Maximum field lengths
Consider adding additional test cases or verifying if these are covered elsewhere in the test suite.
179-220: LGTM! Comprehensive assertions for UI components.The test thoroughly validates the structure and properties of UI components, including:
- Page definitions and containers
- Table representations
- Action buttons and their configurations
1-110: Well-structured test setup with comprehensive model creation!The test class follows good testing practices with proper separation of concerns, clear model definition, and robust test environment setup.
judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/viewTableDeclarationViewPage.etl (1)
83-94: LGTM! Improved rule execution control.The removal of @greedy annotations in favor of @lazy is a good change as it:
- Provides better control over rule execution
- Prevents unnecessary transformations
- Improves performance by executing rules only when needed
Also applies to: 96-107, 109-118, 120-129
judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiNavigationTest.java (3)
99-100: LGTM! Clean transition from table to row declarations.The change from table to row declarations is consistent with the new data representation model.
Also applies to: 119-120, 123-126
137-142: Verify array notation consistency.The change to array notation (
ProductsRow[]) is correct, but ensure this pattern is consistently applied across all table declarations.Run this script to check for consistency:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check for any remaining table declarations without array notation rg "table \w+[^[]" --type javaLength of output: 119
Re-run Consistency Verification for Array Notation in Table Declarations
The original regex produced an error due to an unclosed character class. To verify that every table declaration uses array notation consistently, please run the updated script below. This version first finds all table declarations and then filters out those that already include "[]". If no output is produced, then every table declaration is correctly using array notation.
🏁 Script executed:
#!/bin/bash # Search for table declarations that do not include array notation by: # 1. Finding lines containing "table" followed by a word (the table name) # 2. Filtering out lines where "[]" is present. rg "table\s+\w+" --type java | grep -v "\[\]"Length of output: 443
Consistency in Array Notation Verified
The updated script filtered out all table declarations missing the array notation. The only output lines were from a file using widget declarations—which aren’t relevant to table declarations—confirming that no table declaration is missing the proper "[]" array notation.
100-120: Clean transition to row-based declarations.The changes effectively transition from table to row declarations while maintaining test integrity and backward compatibility.
judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiApplicationTest.java (3)
119-126: LGTM! Consistent row declarations.The changes to row declarations are consistent across all entities and follow the same pattern.
Also applies to: 251-259
269-274: Consider adding tests for row-specific functionality.While the table to row transition is complete, consider adding tests for row-specific features:
- Row selection behavior
- Row-level actions
- Row data binding
Consider adding these test cases or verify if they're covered elsewhere.
Also applies to: 276-281
119-126: Consistent implementation of row-based declarations.The changes effectively implement the new row-based pattern across all test cases while maintaining comprehensive test coverage.
Also applies to: 251-259
judo-tatami-jsl-jsl2ui/src/test/java/hu/blackbelt/judo/tatami/jsl/jsl2ui/application/JslModel2UiWidgetsTest.java (7)
407-409: LGTM! The change aligns with the row-based representation.The renaming from
UserTabletoUserRowis consistent with the PR's objective of transforming table card representation.
415-415: LGTM! The selector update follows the row-based pattern.The selector update from
RelatedTabletoRelatedRowmaintains consistency with the row-based representation.
424-424: LGTM! The table declaration uses the row-based array type.The update to use
RelatedRow[]array type is consistent with the row-based representation.
436-436: LGTM! The table declaration follows the row-based pattern.The update to use
RelatedRow[]array type maintains consistency with the row-based representation.
439-442: LGTM! The entity definition follows the row-based pattern.The renaming from
RelatedTabletoRelatedRowand its column definitions are consistent with the row-based representation.
461-461: LGTM! The menu table declaration uses the row-based array type.The update to use
UserRow[]array type in the menu maintains consistency with the row-based representation.
537-538: LGTM! The test assertions are updated for the row-based pattern.The test assertions are correctly updated to use the new row-based names, maintaining consistency with the transformation.
Also applies to: 555-559, 562-567
pom.xml (1)
67-67: Verify compatibility with the updated judo-meta-jsl version.The
judo-meta-jslversion has been updated to a newer build. Please ensure that this version is compatible with the table to row transformation changes.Run the following script to check for any breaking changes in the dependency:
❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Description: Check for breaking changes in judo-meta-jsl dependency. # Test: Check if the old version exists in the Maven repository curl -s "https://nexus.judo.technology/repository/maven-judong-snapshots/hu/blackbelt/judo/meta/hu.blackbelt.judo.meta.jsl.model/1.0.3.20250203_210405_519d7173_develop/" # Test: Check if the new version exists in the Maven repository curl -s "https://nexus.judo.technology/repository/maven-judong-snapshots/hu/blackbelt/judo/meta/hu.blackbelt.judo.meta.jsl.model/1.0.3.20250211_122715_0d81c688_develop/"Length of output: 4905
Compatibility check for updated judo-meta-jsl version
Both the old and new versions are present in the Nexus repository (as confirmed by the curl queries), and there’s no immediate indication of a breaking change from the repository output. However, the HTML responses are generic (indicating that the repository isn’t directly browseable) and do not provide detailed metadata about the changes. Therefore, while the artifact upload appears correct, please perform additional integration or functional tests to ensure that the new build is fully compatible with the table to row transformation changes.
judo-tatami-jsl-jsl2ui/src/main/epsilon/operations/jsl/ui/cardDeclaration.eol
Show resolved
Hide resolved
judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/cardDeclaration.etl
Show resolved
Hide resolved
judo-tatami-jsl-jsl2ui/src/main/epsilon/transformations/ui/modules/view/rowDeclaration.etl
Show resolved
Hide resolved
noherczeg
deleted the
feature/JNG-6169-transform-cards-table-representation
branch
Followup for: BlackBeltTechnology/judo-meta-jsl@0d81c68