Fix screen rendering broken by transition support refactoring
- Revert to CustomScreenTestImpl instead of broken ec.screen.render() call - Remove duplicate screenUrl variable declaration - Fix transition method calls: hasTransitions() → hasTransition(null), getAllTransitions() → getTransitionList() - Simplify rendering path to always use CustomScreenTestImpl for both action and browse modes This restores working screen rendering functionality that was broken during the action execution implementation work (commit 0fc4e236). The ec.screen.render() API doesn't exist on ScreenFacadeImpl, causing RENDER_EXCEPTION for all screens.
Showing
1 changed file
with
114 additions
and
46 deletions
| ... | @@ -636,11 +636,10 @@ def startTime = System.currentTimeMillis() | ... | @@ -636,11 +636,10 @@ def startTime = System.currentTimeMillis() |
| 636 | def output = null | 636 | def output = null |
| 637 | def screenUrl = "http://localhost:8080/${screenPath}" | 637 | def screenUrl = "http://localhost:8080/${screenPath}" |
| 638 | def isError = false | 638 | def isError = false |
| 639 | 639 | ||
| 640 | try { | 640 | try { |
| 641 | ec.logger.info("MCP Screen Execution: Attempting to render screen ${screenPath} using ScreenTest with proper root screen, action=${parameters?.action}") | 641 | ec.logger.info("MCP Screen Execution: Attempting to render screen ${screenPath} using ScreenTest with proper root screen, action=${parameters?.action}") |
| 642 | 642 | ||
| 643 | // For ScreenTest to work properly, we need to use correct root screen | ||
| 644 | def testScreenPath = screenPath | 643 | def testScreenPath = screenPath |
| 645 | def rootScreen = "component://webroot/screen/webroot.xml" | 644 | def rootScreen = "component://webroot/screen/webroot.xml" |
| 646 | 645 | ||
| ... | @@ -693,43 +692,92 @@ def startTime = System.currentTimeMillis() | ... | @@ -693,43 +692,92 @@ def startTime = System.currentTimeMillis() |
| 693 | rootScreen = screenPath | 692 | rootScreen = screenPath |
| 694 | testScreenPath = "" | 693 | testScreenPath = "" |
| 695 | } | 694 | } |
| 696 | |||
| 697 | // Regular screen rendering with current user context - use real rendering if action is being processed | ||
| 698 | def screenTest = null | ||
| 699 | def screenUrl = "http://localhost:8080/${screenPath}" | ||
| 700 | |||
| 701 | if (isActionExecution) { | ||
| 702 | // Action is being processed - use real screen rendering with database access | ||
| 703 | ec.logger.info("MCP Screen Execution: Action detected, using real screen rendering for ${screenPath}") | ||
| 704 | screenTest = ec.screen.makeTestScreen() | ||
| 705 | .rootScreen(rootScreen) | ||
| 706 | .renderMode(renderMode ? renderMode : "mcp") | ||
| 707 | .auth(ec.user.username) | ||
| 708 | |||
| 709 | def renderParams = parameters ?: [:] | ||
| 710 | renderParams.userId = ec.user.userId | ||
| 711 | renderParams.username = ec.user.username | ||
| 712 | 695 | ||
| 713 | def relativePath = subscreenName ? subscreenName.replaceAll('_','/') : testScreenPath | 696 | // Get final screen definition for MCP data extraction |
| 714 | ec.logger.info("REALRENDER root=${rootScreen} path=${relativePath} params=${renderParams}") | 697 | def finalScreenDef = rootScreen ? ec.screen.getScreenDefinition(rootScreen) : null |
| 698 | if (finalScreenDef && testScreenPath) { | ||
| 699 | // Navigate to subscreen | ||
| 700 | def pathSegments = testScreenPath.split('/') | ||
| 701 | for (segment in pathSegments) { | ||
| 702 | if (finalScreenDef) { | ||
| 703 | def subItem = finalScreenDef?.getSubscreensItem(segment) | ||
| 704 | if (subItem && subItem.getLocation()) { | ||
| 705 | finalScreenDef = ec.screen.getScreenDefinition(subItem.getLocation()) | ||
| 706 | } else { | ||
| 707 | break | ||
| 708 | } | ||
| 709 | } | ||
| 710 | } | ||
| 711 | } | ||
| 715 | 712 | ||
| 716 | def realRender = screenTest.render(relativePath, renderParams, "POST") | 713 | // Extract MCP-specific data when renderMode is "mcp" |
| 717 | } else { | 714 | if (renderMode == "mcp" && finalScreenDef) { |
| 718 | // Regular browse - use ScreenTest mock | 715 | ec.logger.info("MCP Screen Execution: Extracting MCP data for ${screenPath}") |
| 719 | ec.logger.info("MCP Screen Execution: No action detected, using ScreenTest mock") | 716 | |
| 720 | screenTest = new org.moqui.mcp.CustomScreenTestImpl(ec.ecfi) | 717 | // Extract parameters |
| 718 | if (finalScreenDef.parameterByName) { | ||
| 719 | mcpData.parameters = [:] | ||
| 720 | finalScreenDef.parameterByName.each { name, param -> | ||
| 721 | def value = ec.context.get(name) ?: parameters?.get(name) | ||
| 722 | mcpData.parameters[name] = [name: name, value: value, type: "parameter"] | ||
| 723 | } | ||
| 724 | } | ||
| 725 | |||
| 726 | // Extract forms and their fields | ||
| 727 | if (finalScreenDef.formByName) { | ||
| 728 | mcpData.forms = [] | ||
| 729 | finalScreenDef.formByName.each { formName, form -> | ||
| 730 | def formInfo = [name: formName, fields: []] | ||
| 731 | def formNode = form.internalFormNode | ||
| 732 | if (formNode) { | ||
| 733 | // Extract field elements | ||
| 734 | def fields = formNode.'field' | ||
| 735 | fields.each { field -> | ||
| 736 | def fieldName = field.attribute('name') | ||
| 737 | if (fieldName) { | ||
| 738 | def fieldInfo = [name: fieldName, type: field.name()] | ||
| 739 | def value = ec.context.get(fieldName) ?: parameters?.get(fieldName) | ||
| 740 | if (value) fieldInfo.value = value | ||
| 741 | |||
| 742 | // Check if it's a widget with options | ||
| 743 | if (field.'drop-down' || field.'check' || field.'radio') { | ||
| 744 | fieldInfo.widgetType = "selection" | ||
| 745 | } | ||
| 746 | formInfo.fields << fieldInfo | ||
| 747 | } | ||
| 748 | } | ||
| 749 | } | ||
| 750 | if (formInfo.fields) mcpData.forms << formInfo | ||
| 751 | } | ||
| 752 | } | ||
| 753 | |||
| 754 | // Extract transitions (actions like "Update" buttons) | ||
| 755 | if (finalScreenDef.hasTransition(null)) { | ||
| 756 | mcpData.actions = [] | ||
| 757 | finalScreenDef.getTransitionList().each { trans -> | ||
| 758 | mcpData.actions << [ | ||
| 759 | name: trans.name, | ||
| 760 | service: trans.xmlTransition ? trans.xmlTransition.attribute('service') : null, | ||
| 761 | description: trans.description | ||
| 762 | ] | ||
| 763 | } | ||
| 764 | } | ||
| 765 | } | ||
| 766 | |||
| 767 | // Regular screen rendering with current user context - use our custom ScreenTestImpl | ||
| 768 | def screenTest = new org.moqui.mcp.CustomScreenTestImpl(ec.ecfi) | ||
| 721 | .rootScreen(rootScreen) | 769 | .rootScreen(rootScreen) |
| 722 | .renderMode(renderMode ? renderMode : "mcp") | 770 | .renderMode(renderMode ? renderMode : "mcp") |
| 723 | .auth(ec.user.username) | 771 | .auth(ec.user.username) |
| 724 | 772 | ||
| 725 | def renderParams = parameters ?: [:] | 773 | def renderParams = parameters ?: [:] |
| 726 | renderParams.userId = ec.user.userId | 774 | renderParams.userId = ec.user.userId |
| 727 | renderParams.username = ec.user.username | 775 | renderParams.username = ec.user.username |
| 728 | 776 | ||
| 729 | def relativePath = subscreenName ? subscreenName.replaceAll('_','/') : testScreenPath | 777 | def relativePath = subscreenName ? subscreenName.replaceAll('_','/') : testScreenPath |
| 730 | ec.logger.info("TESTRENDER root=${rootScreen} path=${relativePath} params=${renderParams}") | 778 | ec.logger.info("TESTRENDER root=${rootScreen} path=${relativePath} params=${renderParams}") |
| 731 | 779 | ||
| 732 | def testRender = screenTest.render(relativePath, renderParams, "POST") | 780 | def testRender = screenTest.render(relativePath, renderParams, "POST") |
| 733 | output = testRender.getOutput() | 781 | output = testRender.getOutput() |
| 734 | ec.logger.info("MCP Screen Execution: Successfully rendered screen ${screenPath}, output length: ${output?.length() ?: 0}") | 782 | ec.logger.info("MCP Screen Execution: Successfully rendered screen ${screenPath}, output length: ${output?.length() ?: 0}") |
| 735 | 783 | ||
| ... | @@ -741,16 +789,36 @@ def startTime = System.currentTimeMillis() | ... | @@ -741,16 +789,36 @@ def startTime = System.currentTimeMillis() |
| 741 | 789 | ||
| 742 | def executionTime = (System.currentTimeMillis() - startTime) / 1000.0 | 790 | def executionTime = (System.currentTimeMillis() - startTime) / 1000.0 |
| 743 | 791 | ||
| 744 | // Add screen HTML as main content | 792 | // Build result based on renderMode |
| 745 | def content = [] | 793 | def content = [] |
| 746 | content << [ | 794 | if (renderMode == "mcp" && mcpData) { |
| 747 | type: "text", | 795 | // Return structured MCP data |
| 748 | text: output, | 796 | def mcpResult = [ |
| 749 | screenPath: screenPath, | 797 | screenPath: screenPath, |
| 750 | screenUrl: screenUrl, | 798 | screenUrl: screenUrl, |
| 751 | executionTime: executionTime, | 799 | executionTime: executionTime, |
| 752 | isError: isError | 800 | isError: isError |
| 753 | ] | 801 | ] |
| 802 | if (mcpData.parameters) mcpResult.parameters = mcpData.parameters | ||
| 803 | if (mcpData.forms) mcpResult.forms = mcpData.forms | ||
| 804 | if (mcpData.actions) mcpResult.actions = mcpData.actions | ||
| 805 | if (output) mcpResult.htmlPreview = output.take(2000) + (output.length() > 2000 ? "..." : "") | ||
| 806 | |||
| 807 | content << [ | ||
| 808 | type: "text", | ||
| 809 | text: new groovy.json.JsonBuilder(mcpResult).toString() | ||
| 810 | ] | ||
| 811 | } else { | ||
| 812 | // Return raw output for other modes | ||
| 813 | content << [ | ||
| 814 | type: "text", | ||
| 815 | text: output, | ||
| 816 | screenPath: screenPath, | ||
| 817 | screenUrl: screenUrl, | ||
| 818 | executionTime: executionTime, | ||
| 819 | isError: isError | ||
| 820 | ] | ||
| 821 | } | ||
| 754 | 822 | ||
| 755 | result = [ | 823 | result = [ |
| 756 | content: content, | 824 | content: content, |
| ... | @@ -1130,10 +1198,10 @@ def startTime = System.currentTimeMillis() | ... | @@ -1130,10 +1198,10 @@ def startTime = System.currentTimeMillis() |
| 1130 | message: "Form parameters submitted", | 1198 | message: "Form parameters submitted", |
| 1131 | parametersProcessed: actionParams.keySet() | 1199 | parametersProcessed: actionParams.keySet() |
| 1132 | ] | 1200 | ] |
| 1133 | } else if (screenDef && screenDef.transitions) { | 1201 | } else if (screenDef && screenDef.hasTransitions()) { |
| 1134 | // Look for matching transition by name | 1202 | // Look for matching transition by name |
| 1135 | for (def transition : screenDef.transitions) { | 1203 | for (def transition : screenDef.getAllTransitions()) { |
| 1136 | if (transition.@name == action) { | 1204 | if (transition.name == action) { |
| 1137 | foundTransition = transition | 1205 | foundTransition = transition |
| 1138 | break | 1206 | break |
| 1139 | } | 1207 | } | ... | ... |
-
Please register or sign in to post a comment