[ntp][enterprise] Allow Attachment resourceUrl field to be optional
The Attachment struct is reused by the Google and Outlook calendar cards. This CL prepares for the possibility that the resourceUrl will not exist for Outlook. The Google Calendar module should continue to always have a resourceUrl for attachments.
Calendar event attachments that do not have a resourceUrl will be
disabled. The correct UX specs will be followed in another CL.
TEST=manually tested attachments for Google Calendar card continues to WAI
Bug: 376515087
Change-Id: If88bcc51f437a84b6cb2e20509babada1cb741d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6005552
Commit-Queue: Jennifer Serrano <jennserrano@google.com>
Reviewed-by: Riley Tatum <rtatum@google.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1382342}
diff --git a/chrome/browser/new_tab_page/modules/v2/calendar/calendar_data.mojom b/chrome/browser/new_tab_page/modules/v2/calendar/calendar_data.mojom
index 3dd9ac0..274c439 100644
--- a/chrome/browser/new_tab_page/modules/v2/calendar/calendar_data.mojom
+++ b/chrome/browser/new_tab_page/modules/v2/calendar/calendar_data.mojom
@@ -11,8 +11,8 @@
struct Attachment {
// Name of attachment.
string title;
- // Url to attachment.
- url.mojom.Url resource_url;
+ // Url to attachment, if it exists.
+ url.mojom.Url? resource_url;
// Url to attachment icon.
url.mojom.Url icon_url;
};
diff --git a/chrome/browser/new_tab_page/modules/v2/calendar/calendar_fake_data_helper.cc b/chrome/browser/new_tab_page/modules/v2/calendar/calendar_fake_data_helper.cc
index 2b01efd..b93c42f3 100644
--- a/chrome/browser/new_tab_page/modules/v2/calendar/calendar_fake_data_helper.cc
+++ b/chrome/browser/new_tab_page/modules/v2/calendar/calendar_fake_data_helper.cc
@@ -36,8 +36,10 @@
ntp::calendar::mojom::AttachmentPtr attachment =
ntp::calendar::mojom::Attachment::New();
attachment->title = "Attachment " + base::NumberToString(i);
- attachment->resource_url =
- GURL("https://foo.com/attachment" + base::NumberToString(i));
+ if (calendar_type != CalendarType::OUTLOOK_CALENDAR) {
+ attachment->resource_url =
+ GURL("https://foo.com/attachment" + base::NumberToString(i));
+ }
attachment->icon_url = calendar_type == CalendarType::GOOGLE_CALENDAR
? GURL(kGoogleCalendarDriveIconUrl)
: GURL(kOutlookCalendarDocIconUrl);
diff --git a/chrome/browser/new_tab_page/modules/v2/calendar/outlook_calendar_page_handler_unittest.cc b/chrome/browser/new_tab_page/modules/v2/calendar/outlook_calendar_page_handler_unittest.cc
index 8fbcfc2e..dda66d1 100644
--- a/chrome/browser/new_tab_page/modules/v2/calendar/outlook_calendar_page_handler_unittest.cc
+++ b/chrome/browser/new_tab_page/modules/v2/calendar/outlook_calendar_page_handler_unittest.cc
@@ -56,8 +56,7 @@
ntp::calendar::mojom::AttachmentPtr attachment =
std::move(events[i]->attachments[j]);
EXPECT_EQ(attachment->title, "Attachment " + base::NumberToString(j));
- EXPECT_EQ(attachment->resource_url,
- "https://foo.com/attachment" + base::NumberToString(j));
+ EXPECT_FALSE(attachment->resource_url.has_value());
}
EXPECT_EQ(events[i]->conference_url,
GURL("https://foo.com/conference" + base::NumberToString(i)));
diff --git a/chrome/browser/resources/new_tab_page/modules/v2/calendar/calendar_event.html b/chrome/browser/resources/new_tab_page/modules/v2/calendar/calendar_event.html
index 94f58cc..1b8a6df 100644
--- a/chrome/browser/resources/new_tab_page/modules/v2/calendar/calendar_event.html
+++ b/chrome/browser/resources/new_tab_page/modules/v2/calendar/calendar_event.html
@@ -17,7 +17,8 @@
<div id="attachmentList" class="${this.attachmentListClass_}">
${this.event.attachments.map((item, index) => html`
<cr-chip data-index="${index}" @click="${this.openAttachment_}"
- chip-role="link" class="attachment" title="${item.title}">
+ chip-role="link" class="attachment" title="${item.title}"
+ ?disabled="${this.isAttachmentDisabled_(index)}">
<img is="cr-auto-img" auto-src="${item.iconUrl.url}" alt="">
<div class="attachment-text">${item.title}</div>
</cr-chip>
diff --git a/chrome/browser/resources/new_tab_page/modules/v2/calendar/calendar_event.ts b/chrome/browser/resources/new_tab_page/modules/v2/calendar/calendar_event.ts
index f52591f..bc42f9b8 100644
--- a/chrome/browser/resources/new_tab_page/modules/v2/calendar/calendar_event.ts
+++ b/chrome/browser/resources/new_tab_page/modules/v2/calendar/calendar_event.ts
@@ -139,13 +139,19 @@
'modulesCalendarInXHr', Math.round(hoursUntilMeeting).toString());
}
+ protected isAttachmentDisabled_(index: number): boolean {
+ return !this.event.attachments[index].resourceUrl?.url;
+ }
+
protected openAttachment_(e: Event) {
this.dispatchEvent(new Event('usage', {composed: true, bubbles: true}));
recordCalendarAction(CalendarAction.ATTACHMENT_CLICKED, this.moduleName);
const currentTarget = e.currentTarget as HTMLElement;
const index = Number(currentTarget.dataset['index']);
- WindowProxy.getInstance().navigate(
- this.event.attachments[index]!.resourceUrl.url);
+ const resourceUrl = this.event.attachments[index].resourceUrl?.url;
+ if (resourceUrl) {
+ WindowProxy.getInstance().navigate(resourceUrl);
+ }
}
protected openVideoConference_() {
diff --git a/chrome/test/data/webui/new_tab_page/modules/v2/calendar/calendar_event_test.ts b/chrome/test/data/webui/new_tab_page/modules/v2/calendar/calendar_event_test.ts
index 1bddf8f..e333b44 100644
--- a/chrome/test/data/webui/new_tab_page/modules/v2/calendar/calendar_event_test.ts
+++ b/chrome/test/data/webui/new_tab_page/modules/v2/calendar/calendar_event_test.ts
@@ -198,6 +198,43 @@
assertEquals(attachmentListElement.className, 'scrollable-left');
});
+ test('attachment is disabled when resourceUrl does not exist', async () => {
+ const moduleName = 'OutlookCalendar';
+ element.expanded = true;
+ element.event = createEvent(
+ 1, {attachments: createAttachments(3, {resourceUrl: {url: ''}})});
+ element.moduleName = moduleName;
+ await microtasksFinished();
+
+ const attachments = element.renderRoot!.querySelectorAll('.attachment');
+ assertEquals(attachments.length, 3);
+
+ // Assert attachments are disabled.
+ for (let i = 0; i < attachments.length; i++) {
+ const attachment = attachments[i] as any;
+ assertTrue(!!attachment);
+ assertTrue(attachment.hasAttribute('disabled'));
+ }
+ });
+
+ test('attachment not disabled when resourceUrl exists', async () => {
+ const moduleName = 'OutlookCalendar';
+ element.expanded = true;
+ element.event = createEvent(1, {attachments: createAttachments(3)});
+ element.moduleName = moduleName;
+ await microtasksFinished();
+
+ // Assert.
+ const attachments = element.renderRoot!.querySelectorAll('.attachment');
+ assertEquals(attachments.length, 3);
+
+ for (let i = 0; i < attachments.length; i++) {
+ const attachment = attachments[i] as any;
+ assertTrue(!!attachment);
+ assertFalse(attachment.hasAttribute('disabled'));
+ }
+ });
+
test('conference button hidden if empty', async () => {
element.expanded = true;
element.event = createEvent(1, {conferenceUrl: {url: ''}});
diff --git a/chrome/test/data/webui/new_tab_page/modules/v2/calendar/test_support.ts b/chrome/test/data/webui/new_tab_page/modules/v2/calendar/test_support.ts
index 339b749..e9c216f 100644
--- a/chrome/test/data/webui/new_tab_page/modules/v2/calendar/test_support.ts
+++ b/chrome/test/data/webui/new_tab_page/modules/v2/calendar/test_support.ts
@@ -14,14 +14,17 @@
};
}
-export function createAttachments(num: number): Attachment[] {
+export function createAttachments(
+ num: number, overrides?: Partial<Attachment>): Attachment[] {
const attachments: Attachment[] = [];
for (let i = 0; i < num; i++) {
- attachments.push({
- title: `Attachment ${i}`,
- iconUrl: {url: `https://foo.com/attachment${i}`},
- resourceUrl: {url: `https://foo.com/attachmet${i}`},
- });
+ attachments.push(Object.assign(
+ {
+ title: `Attachment ${i}`,
+ iconUrl: {url: `https://foo.com/attachment${i}`},
+ resourceUrl: {url: `https://foo.com/attachmet${i}`},
+ },
+ overrides));
}
return attachments;
}