fix: rework fix for #631 to be less likely to cause breaking changes

Author: ascott18

CHANGELOG.md

@@ -1,6 +1,6 @@
 # 6.1.0
 - Added support for .NET 10
-- `c-loader-status`: The `initial-content` flag now behaves more consistently. A loader is now considered to have initial content if it has ever successfully invoked. Previously, some types of failures (e.g. network errors) would not clear this state, but other kinds of failures (explicit server error responses) did clear this state. If errors should result in hidden content, the `no-error-content` flag should be used.
+- API Callers now set `hasResult = true` for void-returning endpoints that have successfully loaded. This means that the `no-initial-content` flag on `c-loader-status` works as expected for void-returning endpoints.
 
 # 6.0.4
 - AuditLogging: Stored procedure cache was not being correctly persisted to avoid redundant `SELECT OBJECT_DEFINITION` queries.

docs/stacks/vue/coalesce-vue-vuetify/components/c-loader-status.md

@@ -175,7 +175,7 @@ The available flags are as follows, all of which default to `true` except for `s
 | - | - |
 | `no-loading-content` | Controls whether the default slot is rendered while any API caller is loading (i.e. when  `caller.isLoading === true`). |
 | `no-error-content` | Controls whether the default slot is rendered while any API Caller is in an error state (i.e. when  `caller.wasSuccessful === false`). |
-| `no-initial-content` | Controls whether the default slot is rendered while any API Caller has yet to receive a successful response for the first time. |
+| `no-initial-content` | Controls whether the default slot is rendered while any API Caller does not have a result (i.e. when `caller.hasResult === false`). |
 | `no-progress` | Master toggle for whether the progress indicator is shown in any scenario. |
 | `no-initial-progress` | Controls whether the progress indicator is shown when an API Caller is loading for the very first time (i.e. when  `caller.wasSuccessful === null`). |
 | `no-secondary-progress` | Controls whether the progress indicator is shown when an API Caller is loading any time after its first invocation (i.e. when  `caller.wasSuccessful !== null`). |

docs/stacks/vue/layers/api-clients.md

@@ -216,7 +216,7 @@ A message from the last response. Typically an error message if the last request
 readonly hasResult: boolean
 ```
 
-True if `result` is non-null. This prop can be useful in niche, performance-critical scenarios where checking `result` directly will cause an overabundance of re-renders in high-churn scenarios.
+True if `result` is non-null. This is also true for void-returning endpoints that were successful.
 
 ### args {#args}
 

src/IntelliTect.Coalesce.Tests/TargetClasses/TestDbContext/ComplexModel.cs

@@ -339,6 +339,12 @@ public static ItemResult<IFile> DownloadAttachmentStatic()
         return new File(new byte[] { 0x42 }) { Name = "42.png" };
     }
 
+    [Coalesce]
+    public ListResult<ComplexModel> ReturnsListResult()
+    {
+        return new();
+    }
+
     [ControllerAction(Method = HttpMethod.Get)]
     [Coalesce]
     [SemanticKernel("MethodWithOptionalCancellationToken")]

src/coalesce-vue-vuetify3/src/components/display/c-loader-status.spec.tsx

@@ -1,6 +1,7 @@
 import { mountApp, mockEndpoint, flushPromises } from "@test/util";
 import CLS from "./c-loader-status.vue";
 import { ComplexModelViewModel } from "@test-targets/viewmodels.g";
+import { AxiosError } from "axios";
 
 describe("CLoaderStatus", () => {
   const vm = new ComplexModelViewModel();
@@ -145,61 +146,152 @@ describe("CLoaderStatus", () => {
     expect(wrapper.text()).toContain("Success");
   });
 
-  test("void-returning endpoint with no-initial-content shows content after success then failure", async () => {
-    // This tests the fix for void-returning endpoints where hasResult should be true
-    // when the endpoint succeeds, even though result is null.
-    const vm = new ComplexModelViewModel();
-    vm.$primaryKey = 1;
-    const voidCaller = vm.methodWithOptionalEnumParam;
-
-    const mockSuccess = mockEndpoint(
-      vm.$metadata.methods.methodWithOptionalEnumParam,
-      () => ({
+  const endpoints = [
+    {
+      name: "void-returning",
+      methodName: "methodWithOptionalEnumParam" as const,
+      mockSuccessResponse: () => ({
         wasSuccessful: true,
         // Void endpoints don't have an 'object' property in the response
       }),
-    );
-
-    const wrapper = mountApp(() => (
-      <CLS loaders={voidCaller} no-initial-content>
-        <div>Content</div>
-      </CLS>
-    ));
-
-    // Initially, content should be hidden (wasSuccessful is null)
-    expect(wrapper.text()).not.toContain("Content");
-
-    // Call the void endpoint successfully
-    await voidCaller();
-    await flushPromises();
-
-    // After success, content should be visible
-    expect(voidCaller.wasSuccessful).toBe(true);
-    expect(voidCaller.result).toBeNull();
-    expect(voidCaller._hasLoaded.value).toBe(true);
-    expect(wrapper.text()).toContain("Content");
-
-    mockSuccess.destroy();
-
-    // Now mock a failure
-    const mockFailure = mockEndpoint(
-      vm.$metadata.methods.methodWithOptionalEnumParam,
-      () => ({
-        wasSuccessful: false,
-        message: "Error occurred",
+    },
+    {
+      name: "string-returning",
+      methodName: "methodWithOptionalParams" as const,
+      mockSuccessResponse: () => ({
+        wasSuccessful: true,
+        object: "test string result",
       }),
-    );
-
-    // Call the endpoint again, this time it fails
-    await expect(voidCaller()).rejects.toThrow();
-    await flushPromises();
-
-    // Content should still be visible because hasLoaded is true
-    // (the endpoint has successfully loaded before, even though this call failed)
-    expect(voidCaller.wasSuccessful).toBe(false);
-    expect(voidCaller._hasLoaded.value).toBe(true);
-    expect(wrapper.text()).toContain("Content");
-
-    mockFailure.destroy();
-  });
+    },
+    {
+      name: "object-returning",
+      methodName: "methodWithManyParams" as const,
+      mockSuccessResponse: () => ({
+        wasSuccessful: true,
+        object: { externalParentId: 42, name: "Test Parent" },
+      }),
+    },
+    {
+      name: "ListResult-returning",
+      methodName: "returnsListResult" as const,
+      mockSuccessResponse: () => ({
+        wasSuccessful: true,
+        list: [],
+      }),
+    },
+  ] as const;
+
+  describe.each(endpoints)(
+    "$name endpoint",
+    function ({ methodName, mockSuccessResponse }) {
+      test("no-initial-content shows content after success then Network Error", async () => {
+        // This tests that hasResult is properly set when an endpoint succeeds,
+        // regardless of the return type (void, primitive, object, ItemResult).
+        const vm = new ComplexModelViewModel();
+        vm.$primaryKey = 1;
+        const caller = vm[methodName];
+
+        const mockSuccess = mockEndpoint(
+          vm.$metadata.methods[methodName],
+          mockSuccessResponse,
+        );
+
+        const wrapper = mountApp(() => (
+          <CLS loaders={caller} no-initial-content>
+            <div>Content</div>
+          </CLS>
+        ));
+
+        // Initially, content should be hidden (wasSuccessful is null)
+        expect(wrapper.text()).not.toContain("Content");
+
+        // Call the endpoint successfully
+        await caller();
+        await flushPromises();
+
+        // After success, content should be visible
+        expect(caller.wasSuccessful).toBe(true);
+        expect(caller.hasResult).toBe(true);
+        expect(wrapper.text()).toContain("Content");
+
+        mockSuccess.destroy();
+
+        // Now mock a failure
+        const mockFailure = mockEndpoint(
+          vm.$metadata.methods[methodName],
+          () => {
+            throw new AxiosError("Network Error");
+          },
+        );
+
+        // Call the endpoint again, this time it fails
+        await expect(caller()).rejects.toThrow();
+        await flushPromises();
+
+        // Content should still be visible because Network errors do not wipe a caller's result.
+        // This is tested for because its how Coalesce has always worked,
+        // and would cause all kinds of subtile behavior changes if it ever stopped doing that.
+        expect(caller.wasSuccessful).toBe(false);
+        expect(caller.hasResult).toBe(true);
+        expect(wrapper.text()).toContain("Content");
+
+        mockFailure.destroy();
+      });
+
+      test("no-initial-content hides content after explicit failure", async () => {
+        // This tests that hasResult is properly set when an endpoint succeeds,
+        // regardless of the return type (void, primitive, object, ItemResult).
+        const vm = new ComplexModelViewModel();
+        vm.$primaryKey = 1;
+        const caller = vm[methodName];
+
+        const mockSuccess = mockEndpoint(
+          vm.$metadata.methods[methodName],
+          mockSuccessResponse,
+        );
+
+        const wrapper = mountApp(() => (
+          <CLS loaders={caller} no-initial-content>
+            <div>Content</div>
+          </CLS>
+        ));
+
+        // Initially, content should be hidden (wasSuccessful is null)
+        expect(wrapper.text()).not.toContain("Content");
+
+        // Call the endpoint successfully
+        await caller();
+        await flushPromises();
+
+        // After success, content should be visible
+        expect(caller.wasSuccessful).toBe(true);
+        expect(caller.hasResult).toBe(true);
+        expect(wrapper.text()).toContain("Content");
+
+        mockSuccess.destroy();
+
+        // Now mock a failure
+        const mockFailure = mockEndpoint(
+          vm.$metadata.methods[methodName],
+          () => ({
+            wasSuccessful: false,
+            message: "Explicit failure",
+          }),
+        );
+
+        // Call the endpoint again, this time it fails
+        await expect(caller()).rejects.toThrow();
+        await flushPromises();
+
+        // Since no-initial-status is based on `hasResult`,
+        // and our result has been wiped by an explicit server error, the content should hide.
+        expect(caller.wasSuccessful).toBe(false);
+        expect(caller.hasResult).toBe(false);
+        expect(caller.result).toBeFalsy();
+        expect(wrapper.text()).not.toContain("Content");
+
+        mockFailure.destroy();
+      });
+    },
+  );
 });

src/coalesce-vue-vuetify3/src/components/display/c-loader-status.vue

@@ -341,7 +341,10 @@ const showContent = computed(() => {
       // initial content is off, and either:
       !flags["initial-content"] &&
       // loader has not yet loaded
-      !loader._hasLoaded.value
+      (loader.wasSuccessful == null ||
+        // or loader has loaded, but it errored and there's no current result
+        // (implying it has never successfully loaded).
+        (loader.wasSuccessful === false && !loader.hasResult))
     ) {
       return false;
     }

src/coalesce-vue/src/api-client.ts

@@ -1394,26 +1394,12 @@ export abstract class ApiState<
     this.__isLoading.value = v;
   }
 
-  /**
-   * @internal
-   * Whether the endpoint has ever successfully loaded.
-   * This is internal state because we don't want to confuse users into using it incorrectly
-   * (it's somewhat similar to wasSuccessful and hasResult)
-   * Could expose if there's a compelling argument for doing so.
-   */
-  _hasLoaded = ref(false);
-
   private readonly __wasSuccessful = ref<boolean | null>(null);
   /** True if the previous request was successful. */
   get wasSuccessful() {
     return this.__wasSuccessful.value;
   }
   set wasSuccessful(v) {
-    if (v === null) {
-      this._hasLoaded.value = false;
-    } else if (v) {
-      this._hasLoaded.value = true;
-    }
     this.__wasSuccessful.value = v;
   }
 
@@ -1454,7 +1440,6 @@ export abstract class ApiState<
   public reset() {
     if (this.isLoading)
       throw new Error("Cannot reset while a request is pending.");
-    this.hasResult = false;
     this.result = null;
     this.wasSuccessful = null;
     this.message = null;
@@ -1963,7 +1948,11 @@ export class ItemApiState<
   }
   set result(v) {
     this.__result.value = v;
-    this.hasResult = v != null;
+    if (this.$metadata?.return.type == "void") {
+      this.hasResult = !!this.wasSuccessful;
+    } else {
+      this.hasResult = v != null;
+    }
   }
 
   override get rawResponse() {
@@ -2064,7 +2053,6 @@ export class ItemApiState<
     } else {
       this.result = null;
     }
-    this.hasResult = this.result != null;
   }
 }
 
@@ -2208,6 +2196,7 @@ export class ListApiState<
   }
   set result(v) {
     this.__result.value = v;
+    this.hasResult = v != null;
   }
 
   override get rawResponse() {
@@ -2248,7 +2237,6 @@ export class ListApiState<
     } else {
       this.result = null;
     }
-    this.hasResult = this.result != null;
   }
 }