Fix validation error reporting and clarify feature service docs
Critical fix: Errors now correctly captured and reported - Capture errors immediately after render() in CustomScreenTestImpl - Added saveMessagesToSession() to WebFacadeStub for framework compatibility - Actions that fail validation now report status:'error' instead of false success Documentation fix: Clarify two different feature services - applyProductFeatures: Apply EXISTING features to products (most common) - createProductFeature: Create NEW feature definitions (rare) - Updated EditProduct and ProductFeature wiki docs with correct examples
Showing
3 changed files
with
153 additions
and
24 deletions
| ... | @@ -762,13 +762,16 @@ Manage product details, features, prices, categories, and associations. | ... | @@ -762,13 +762,16 @@ Manage product details, features, prices, categories, and associations. |
| 762 | 762 | ||
| 763 | Features are managed directly on the EditProduct screen (not a separate EditFeatures subscreen). | 763 | Features are managed directly on the EditProduct screen (not a separate EditFeatures subscreen). |
| 764 | 764 | ||
| 765 | **IMPORTANT:** Use `applyProductFeatures` to add existing features (like ColorGreen). | ||
| 766 | Use `createProductFeature` only to create NEW feature definitions. | ||
| 767 | |||
| 765 | ### Add Selectable Feature (Virtual Products) | 768 | ### Add Selectable Feature (Virtual Products) |
| 766 | ``` | 769 | ``` |
| 767 | path: PopCommerce/PopCommerceAdmin/Catalog/Product/EditProduct | 770 | path: PopCommerce/PopCommerceAdmin/Catalog/Product/EditProduct |
| 768 | action: createProductFeature | 771 | action: applyProductFeatures |
| 769 | parameters: { | 772 | parameters: { |
| 770 | productId: "DEMO_VAR", | 773 | productId: "DEMO_VAR", |
| 771 | productFeatureId: "ColorRed", | 774 | productFeatureIdList: ["ColorGreen"], |
| 772 | applTypeEnumId: "PfatSelectable" | 775 | applTypeEnumId: "PfatSelectable" |
| 773 | } | 776 | } |
| 774 | ``` | 777 | ``` |
| ... | @@ -776,10 +779,10 @@ parameters: { | ... | @@ -776,10 +779,10 @@ parameters: { |
| 776 | ### Add Distinguishing Feature (Variant Products) | 779 | ### Add Distinguishing Feature (Variant Products) |
| 777 | ``` | 780 | ``` |
| 778 | path: PopCommerce/PopCommerceAdmin/Catalog/Product/EditProduct | 781 | path: PopCommerce/PopCommerceAdmin/Catalog/Product/EditProduct |
| 779 | action: createProductFeature | 782 | action: applyProductFeatures |
| 780 | parameters: { | 783 | parameters: { |
| 781 | productId: "DEMO_VAR_RED", | 784 | productId: "DEMO_VAR_GR_SM", |
| 782 | productFeatureId: "ColorRed", | 785 | productFeatureIdList: ["ColorGreen", "SizeSmall"], |
| 783 | applTypeEnumId: "PfatDistinguishing" | 786 | applTypeEnumId: "PfatDistinguishing" |
| 784 | } | 787 | } |
| 785 | ``` | 788 | ``` |
| ... | @@ -944,14 +947,31 @@ parameters: { | ... | @@ -944,14 +947,31 @@ parameters: { |
| 944 | versionName="v1"> | 947 | versionName="v1"> |
| 945 | <fileData><![CDATA[# ProductFeature Service | 948 | <fileData><![CDATA[# ProductFeature Service |
| 946 | 949 | ||
| 947 | Apply features (like color, size) to products. | 950 | Manage product features (color, size, etc.) - two different operations: |
| 948 | 951 | ||
| 949 | ## mantle.product.ProductServices.create#ProductFeature | 952 | ## IMPORTANT: Two Different Services |
| 953 | |||
| 954 | 1. **applyProductFeatures** - Apply EXISTING features to a product (most common) | ||
| 955 | 2. **createProductFeature** - Create a NEW feature definition (rare) | ||
| 956 | |||
| 957 | Most of the time you want `applyProductFeatures` to link existing features like ColorGreen or SizeLarge to products. | ||
| 958 | |||
| 959 | --- | ||
| 960 | |||
| 961 | ## mantle.product.ProductServices.apply#ProductFeatures | ||
| 962 | |||
| 963 | **Use this to apply existing features to products.** | ||
| 964 | |||
| 965 | **Action:** `applyProductFeatures` | ||
| 950 | 966 | ||
| 951 | **Required:** | 967 | **Required:** |
| 952 | - `productId`: Product to add feature to | 968 | - `productId`: Product to add features to |
| 953 | - `productFeatureId`: Feature ID (e.g., ColorRed, SizeMedium) | 969 | - `productFeatureIdList`: List of feature IDs (e.g., ["ColorGreen", "SizeMedium"]) |
| 954 | - `applTypeEnumId`: How feature applies to product | 970 | |
| 971 | **Optional:** | ||
| 972 | - `applTypeEnumId`: How feature applies (default: PfatStandard) | ||
| 973 | - `fromDate`: When feature becomes active (default: now) | ||
| 974 | - `sequenceNum`: Display order | ||
| 955 | 975 | ||
| 956 | **Application Types (applTypeEnumId):** | 976 | **Application Types (applTypeEnumId):** |
| 957 | - `PfatSelectable`: Customer can choose (use on virtual parent) | 977 | - `PfatSelectable`: Customer can choose (use on virtual parent) |
| ... | @@ -959,34 +979,61 @@ Apply features (like color, size) to products. | ... | @@ -959,34 +979,61 @@ Apply features (like color, size) to products. |
| 959 | - `PfatStandard`: Always included | 979 | - `PfatStandard`: Always included |
| 960 | - `PfatRequired`: Must be selected at purchase | 980 | - `PfatRequired`: Must be selected at purchase |
| 961 | 981 | ||
| 962 | **Optional:** | 982 | **Example - Add Selectable Feature to Virtual Parent:** |
| 963 | - `fromDate`: When feature becomes active (default: now) | ||
| 964 | - `sequenceNum`: Display order | ||
| 965 | |||
| 966 | **Example - Virtual Parent:** | ||
| 967 | ``` | 983 | ``` |
| 968 | action: createProductFeature | 984 | action: applyProductFeatures |
| 969 | parameters: { | 985 | parameters: { |
| 970 | productId: "DEMO_VAR", | 986 | productId: "DEMO_VAR", |
| 971 | productFeatureId: "ColorGreen", | 987 | productFeatureIdList: ["ColorGreen"], |
| 972 | applTypeEnumId: "PfatSelectable" | 988 | applTypeEnumId: "PfatSelectable" |
| 973 | } | 989 | } |
| 974 | ``` | 990 | ``` |
| 975 | 991 | ||
| 976 | **Example - Variant:** | 992 | **Example - Add Distinguishing Feature to Variant:** |
| 977 | ``` | 993 | ``` |
| 978 | action: createProductFeature | 994 | action: applyProductFeatures |
| 979 | parameters: { | 995 | parameters: { |
| 980 | productId: "DEMO_VAR_GR_SM", | 996 | productId: "DEMO_VAR_GR_SM", |
| 981 | productFeatureId: "ColorGreen", | 997 | productFeatureIdList: ["ColorGreen", "SizeSmall"], |
| 982 | applTypeEnumId: "PfatDistinguishing" | 998 | applTypeEnumId: "PfatDistinguishing" |
| 983 | } | 999 | } |
| 984 | ``` | 1000 | ``` |
| 985 | 1001 | ||
| 986 | ## Common Feature IDs | 1002 | --- |
| 1003 | |||
| 1004 | ## mantle.product.ProductServices.create#ProductFeature | ||
| 1005 | |||
| 1006 | **Use this ONLY to create a NEW feature that doesn't exist yet.** | ||
| 1007 | |||
| 1008 | **Action:** `createProductFeature` | ||
| 1009 | |||
| 1010 | **Required:** | ||
| 1011 | - `productFeatureTypeEnumId`: Type of feature (PftColor, PftSize, etc.) | ||
| 1012 | - `description`: Feature name/description (e.g., "Green", "Extra Small") | ||
| 1013 | |||
| 1014 | **Optional:** | ||
| 1015 | - `productId`: If provided, also applies the new feature to this product | ||
| 1016 | - `applTypeEnumId`: How to apply (if productId provided) | ||
| 1017 | - `abbrev`: Short abbreviation (e.g., "GR", "XS") | ||
| 1018 | |||
| 1019 | **Example - Create a new color feature:** | ||
| 1020 | ``` | ||
| 1021 | action: createProductFeature | ||
| 1022 | parameters: { | ||
| 1023 | productFeatureTypeEnumId: "PftColor", | ||
| 1024 | description: "Teal", | ||
| 1025 | abbrev: "TL", | ||
| 1026 | productId: "DEMO_VAR", | ||
| 1027 | applTypeEnumId: "PfatSelectable" | ||
| 1028 | } | ||
| 1029 | ``` | ||
| 1030 | |||
| 1031 | --- | ||
| 1032 | |||
| 1033 | ## Common Feature IDs (already exist - use with applyProductFeatures) | ||
| 987 | 1034 | ||
| 988 | Colors: ColorRed, ColorBlue, ColorGreen, ColorBlack, ColorWhite | 1035 | **Colors:** ColorRed, ColorBlue, ColorGreen, ColorBlack, ColorWhite |
| 989 | Sizes: SizeSmall, SizeMedium, SizeLarge, SizeXL]]></fileData> | 1036 | **Sizes:** SizeSmall, SizeMedium, SizeLarge, SizeXL]]></fileData> |
| 990 | </moqui.resource.DbResourceFile> | 1037 | </moqui.resource.DbResourceFile> |
| 991 | 1038 | ||
| 992 | <moqui.resource.wiki.WikiPage | 1039 | <moqui.resource.wiki.WikiPage | ... | ... |
| ... | @@ -397,6 +397,25 @@ class CustomScreenTestImpl implements McpScreenTest { | ... | @@ -397,6 +397,25 @@ class CustomScreenTestImpl implements McpScreenTest { |
| 397 | try { | 397 | try { |
| 398 | logger.info("Starting render for ${screenPathList} with root ${csti.rootScreenLocation}") | 398 | logger.info("Starting render for ${screenPathList} with root ${csti.rootScreenLocation}") |
| 399 | screenRender.render(wfs.getRequest(), wfs.getResponse()) | 399 | screenRender.render(wfs.getRequest(), wfs.getResponse()) |
| 400 | |||
| 401 | // IMMEDIATELY capture errors after render - before anything can clear them | ||
| 402 | // This is critical for transition validation errors which get cleared during redirect handling | ||
| 403 | if (eci.message.hasError()) { | ||
| 404 | List<String> immediateErrors = new ArrayList<>(eci.message.getErrors()) | ||
| 405 | if (immediateErrors.size() > 0) { | ||
| 406 | stri.errorMessages.addAll(immediateErrors) | ||
| 407 | logger.warn("Captured ${immediateErrors.size()} errors immediately after render: ${immediateErrors}") | ||
| 408 | } | ||
| 409 | } | ||
| 410 | List<org.moqui.context.ValidationError> immediateValErrors = eci.message.getValidationErrors() | ||
| 411 | if (immediateValErrors != null && immediateValErrors.size() > 0) { | ||
| 412 | for (org.moqui.context.ValidationError ve : immediateValErrors) { | ||
| 413 | String errMsg = ve.getMessage() ?: "${ve.getField()}: Validation error" | ||
| 414 | stri.errorMessages.add(errMsg) | ||
| 415 | } | ||
| 416 | logger.warn("Captured ${immediateValErrors.size()} validation errors immediately after render") | ||
| 417 | } | ||
| 418 | |||
| 400 | // get the response text from the WebFacadeStub | 419 | // get the response text from the WebFacadeStub |
| 401 | stri.outputString = wfs.getResponseText() | 420 | stri.outputString = wfs.getResponseText() |
| 402 | stri.jsonObj = wfs.getResponseJsonObj() | 421 | stri.jsonObj = wfs.getResponseJsonObj() |
| ... | @@ -416,7 +435,7 @@ class CustomScreenTestImpl implements McpScreenTest { | ... | @@ -416,7 +435,7 @@ class CustomScreenTestImpl implements McpScreenTest { |
| 416 | // pop the context stack, get rid of var space | 435 | // pop the context stack, get rid of var space |
| 417 | cs.pop() | 436 | cs.pop() |
| 418 | 437 | ||
| 419 | // check, pass through, error messages | 438 | // check, pass through, error messages from ExecutionContext |
| 420 | if (eci.message.hasError()) { | 439 | if (eci.message.hasError()) { |
| 421 | stri.errorMessages.addAll(eci.message.getErrors()) | 440 | stri.errorMessages.addAll(eci.message.getErrors()) |
| 422 | eci.message.clearErrors() | 441 | eci.message.clearErrors() |
| ... | @@ -425,6 +444,29 @@ class CustomScreenTestImpl implements McpScreenTest { | ... | @@ -425,6 +444,29 @@ class CustomScreenTestImpl implements McpScreenTest { |
| 425 | logger.warn(sb.toString()) | 444 | logger.warn(sb.toString()) |
| 426 | csti.errorCount += stri.errorMessages.size() | 445 | csti.errorCount += stri.errorMessages.size() |
| 427 | } | 446 | } |
| 447 | |||
| 448 | // Also check errors saved to session by saveMessagesToSession() during transition redirects | ||
| 449 | // This captures validation errors that occur during service calls but get cleared before we check | ||
| 450 | if (wfs instanceof WebFacadeStub) { | ||
| 451 | def savedErrors = wfs.getSavedErrors() | ||
| 452 | if (savedErrors && savedErrors.size() > 0) { | ||
| 453 | stri.errorMessages.addAll(savedErrors) | ||
| 454 | StringBuilder sb = new StringBuilder("Saved error messages from ${stri.screenPath}: ") | ||
| 455 | for (String errorMessage in savedErrors) sb.append("\n").append(errorMessage) | ||
| 456 | logger.warn(sb.toString()) | ||
| 457 | csti.errorCount += savedErrors.size() | ||
| 458 | } | ||
| 459 | def savedValidationErrors = wfs.getSavedValidationErrors() | ||
| 460 | if (savedValidationErrors && savedValidationErrors.size() > 0) { | ||
| 461 | // Convert ValidationError objects to string messages | ||
| 462 | for (def ve in savedValidationErrors) { | ||
| 463 | def errMsg = ve.message ?: "${ve.field}: Validation error" | ||
| 464 | stri.errorMessages.add(errMsg) | ||
| 465 | csti.errorCount++ | ||
| 466 | } | ||
| 467 | logger.warn("Saved validation errors from ${stri.screenPath}: ${savedValidationErrors.size()} errors") | ||
| 468 | } | ||
| 469 | } | ||
| 428 | 470 | ||
| 429 | // check for error strings in output | 471 | // check for error strings in output |
| 430 | if (stri.outputString != null) for (String errorStr in csti.errorStrings) if (stri.outputString.contains(errorStr)) { | 472 | if (stri.outputString != null) for (String errorStr in csti.errorStrings) if (stri.outputString.contains(errorStr)) { | ... | ... |
| ... | @@ -294,6 +294,46 @@ class WebFacadeStub implements WebFacade { | ... | @@ -294,6 +294,46 @@ class WebFacadeStub implements WebFacade { |
| 294 | this.responseText = "System message not implemented in stub" | 294 | this.responseText = "System message not implemented in stub" |
| 295 | } | 295 | } |
| 296 | 296 | ||
| 297 | // Save methods - capture errors/messages before redirect so they're available after render | ||
| 298 | void saveMessagesToSession() { | ||
| 299 | // Capture current errors and validation errors from ExecutionContext | ||
| 300 | if (ecfi instanceof org.moqui.impl.context.ExecutionContextFactoryImpl) { | ||
| 301 | org.moqui.impl.context.ExecutionContextFactoryImpl ecfiImpl = (org.moqui.impl.context.ExecutionContextFactoryImpl) ecfi | ||
| 302 | org.moqui.context.ExecutionContext eci = ecfiImpl.getEci() | ||
| 303 | if (eci != null) { | ||
| 304 | MessageFacade mf = eci.getMessage() | ||
| 305 | if (mf != null) { | ||
| 306 | List<String> errors = mf.getErrors() | ||
| 307 | if (errors != null && errors.size() > 0) { | ||
| 308 | savedErrors.addAll(errors) | ||
| 309 | logger.info("WebFacadeStub.saveMessagesToSession: Captured ${errors.size()} errors: ${errors}") | ||
| 310 | } | ||
| 311 | List<ValidationError> valErrors = mf.getValidationErrors() | ||
| 312 | if (valErrors != null && valErrors.size() > 0) { | ||
| 313 | savedValidationErrors.addAll(valErrors) | ||
| 314 | logger.info("WebFacadeStub.saveMessagesToSession: Captured ${valErrors.size()} validation errors") | ||
| 315 | } | ||
| 316 | } | ||
| 317 | } | ||
| 318 | } | ||
| 319 | } | ||
| 320 | |||
| 321 | void saveRequestParametersToSession() { | ||
| 322 | // Store parameters for potential re-display | ||
| 323 | sessionAttributes.put("moqui.saved.parameters", new HashMap(parameters)) | ||
| 324 | } | ||
| 325 | |||
| 326 | void saveErrorParametersToSession() { | ||
| 327 | // Store error parameters | ||
| 328 | errorParameters.putAll(parameters) | ||
| 329 | } | ||
| 330 | |||
| 331 | void saveParametersToSession(Map parameters) { | ||
| 332 | if (parameters) { | ||
| 333 | sessionAttributes.put("moqui.saved.parameters", new HashMap(parameters)) | ||
| 334 | } | ||
| 335 | } | ||
| 336 | |||
| 297 | // Helper methods for ScreenTestImpl | 337 | // Helper methods for ScreenTestImpl |
| 298 | String getResponseText() { | 338 | String getResponseText() { |
| 299 | if (responseText != null) { | 339 | if (responseText != null) { | ... | ... |
-
Please register or sign in to post a comment