Copilot commented on code in PR #6771:
URL: https://github.com/apache/texera/pull/6771#discussion_r3627711468
##########
frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts:
##########
@@ -512,4 +512,208 @@ describe("ShareAccessComponent", () => {
expect(notificationSpy.success).toHaveBeenCalledWith("Dataset
unpublished successfully");
});
});
+
+ describe("hasWriteAccess without a resolved email", () => {
+ it("returns false when the current user has no email at all", () => {
+ // an empty string makes the stubbed UserService.getCurrentUser return
undefined,
+ // so this.currentEmail resolves to undefined and the early-return guard
is exercised
+ const c = setupComponent({ currentEmail: "" });
+ expect(c.currentEmail).toBeUndefined();
+ expect(c.hasWriteAccess).toBe(false);
+ });
+ });
+
+ describe("removeEmailTag", () => {
+ it("removes the matching email and keeps the others", () => {
+ const c = setupComponent();
+ c.emailTags = ["[email protected]", "[email protected]",
"[email protected]"];
+ c.removeEmailTag("[email protected]");
+ expect(c.emailTags).toEqual(["[email protected]", "[email protected]"]);
+ });
+
+ it("leaves tags unchanged when the email is not present", () => {
+ const c = setupComponent();
+ c.emailTags = ["[email protected]"];
+ c.removeEmailTag("[email protected]");
+ expect(c.emailTags).toEqual(["[email protected]"]);
+ });
+ });
+
+ describe("onChange", () => {
+ it("filters allOwners case-insensitively by the typed value", () => {
+ const c = setupComponent();
+ (c as any).allOwners = ["Alice", "Bob", "alfred"];
+ c.onChange("al");
Review Comment:
Avoid casting to `any` to mutate `allOwners`. Since `allOwners` is an array,
you can populate it via `push(...)` (or `splice(...)`) without bypassing the
component's `readonly` typing.
##########
frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts:
##########
@@ -512,4 +512,208 @@ describe("ShareAccessComponent", () => {
expect(notificationSpy.success).toHaveBeenCalledWith("Dataset
unpublished successfully");
});
});
+
+ describe("hasWriteAccess without a resolved email", () => {
+ it("returns false when the current user has no email at all", () => {
+ // an empty string makes the stubbed UserService.getCurrentUser return
undefined,
+ // so this.currentEmail resolves to undefined and the early-return guard
is exercised
+ const c = setupComponent({ currentEmail: "" });
+ expect(c.currentEmail).toBeUndefined();
+ expect(c.hasWriteAccess).toBe(false);
+ });
+ });
+
+ describe("removeEmailTag", () => {
+ it("removes the matching email and keeps the others", () => {
+ const c = setupComponent();
+ c.emailTags = ["[email protected]", "[email protected]",
"[email protected]"];
+ c.removeEmailTag("[email protected]");
+ expect(c.emailTags).toEqual(["[email protected]", "[email protected]"]);
+ });
+
+ it("leaves tags unchanged when the email is not present", () => {
+ const c = setupComponent();
+ c.emailTags = ["[email protected]"];
+ c.removeEmailTag("[email protected]");
+ expect(c.emailTags).toEqual(["[email protected]"]);
+ });
+ });
+
+ describe("onChange", () => {
+ it("filters allOwners case-insensitively by the typed value", () => {
+ const c = setupComponent();
+ (c as any).allOwners = ["Alice", "Bob", "alfred"];
+ c.onChange("al");
+ expect(c.filteredOwners).toEqual(["Alice", "alfred"]);
+ });
+
+ it("clears filteredOwners when the value is null", () => {
+ const c = setupComponent();
+ (c as any).allOwners = ["Alice"];
+ c.filteredOwners = ["stale"];
Review Comment:
Same as above: prefer not to cast to `any` to overwrite `allOwners`.
Populate the existing array (e.g., via `push`) so the test stays type-safe and
less brittle.
##########
frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts:
##########
@@ -512,4 +512,208 @@ describe("ShareAccessComponent", () => {
expect(notificationSpy.success).toHaveBeenCalledWith("Dataset
unpublished successfully");
});
});
+
+ describe("hasWriteAccess without a resolved email", () => {
+ it("returns false when the current user has no email at all", () => {
+ // an empty string makes the stubbed UserService.getCurrentUser return
undefined,
+ // so this.currentEmail resolves to undefined and the early-return guard
is exercised
+ const c = setupComponent({ currentEmail: "" });
+ expect(c.currentEmail).toBeUndefined();
+ expect(c.hasWriteAccess).toBe(false);
+ });
Review Comment:
This test currently depends on a quirk of the spec stub (empty string =>
UserService.getCurrentUser() returns undefined) and then asserts `currentEmail`
is `undefined`. That couples the test to the stub implementation rather than
the component behavior and can break if the stub changes (e.g., returning `{
email: "" }`). Consider making the test explicitly cover the empty-string guard
by setting `currentEmail` directly and only asserting `hasWriteAccess`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
