Skip to content

Commit

Permalink
boca: Implement client delegate for update session.
Browse files Browse the repository at this point in the history
Also include a few fixes:
1. Add in session Id and teacher as they're required for every session
   update request.
2. Only allow update active session.
3. Remove the hardcoded duration.

Test: unit tested.
Bug: b:368071298
Change-Id: Ia1b8b6a288e596a60692456bc6e99a6adc0621cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5879481
Reviewed-by: Benjamin Zielinski <[email protected]>
Commit-Queue: April Zhou <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1358507}
  • Loading branch information
zhouyuzhe authored and Chromium LUCI CQ committed Sep 21, 2024
1 parent ee36e47 commit 98f652c
Show file tree
Hide file tree
Showing 7 changed files with 208 additions and 25 deletions.
7 changes: 4 additions & 3 deletions ash/webui/boca_ui/boca_app_page_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ void BocaAppHandler::GetSession(GetSessionCallback callback) {
void BocaAppHandler::EndSession(EndSessionCallback callback) {
auto* session =
BocaAppClient::Get()->GetSessionManager()->GetCurrentSession();
if (!session) {
if (!session || session->session_state() != ::boca::Session::ACTIVE) {
std::move(callback).Run(mojom::UpdateSessionError::kInvalid);
return;
}
Expand Down Expand Up @@ -256,7 +256,8 @@ void BocaAppHandler::UpdateOnTaskConfig(mojom::OnTaskConfigPtr config,
UpdateOnTaskConfigCallback callback) {
auto* session =
BocaAppClient::Get()->GetSessionManager()->GetCurrentSession();
if (!session || !config) {
if (!session || session->session_state() != ::boca::Session::ACTIVE ||
!config) {
std::move(callback).Run(mojom::UpdateSessionError::kInvalid);
return;
}
Expand Down Expand Up @@ -289,7 +290,7 @@ void BocaAppHandler::UpdateCaptionConfig(mojom::CaptionConfigPtr config,
// Dispatch remote caption config.
auto* session =
BocaAppClient::Get()->GetSessionManager()->GetCurrentSession();
if (!session) {
if (!session || session->session_state() != ::boca::Session::ACTIVE) {
std::move(callback).Run(mojom::UpdateSessionError::kInvalid);
return;
}
Expand Down
48 changes: 48 additions & 0 deletions ash/webui/boca_ui/boca_app_page_handler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,21 @@ TEST_F(BocaAppPageHandlerTest, EndSessionWithEmptyResponse) {
EXPECT_EQ(mojom::UpdateSessionError::kInvalid, future_1.Get().value());
}

TEST_F(BocaAppPageHandlerTest, EndSessionWithNonActiveResponse) {
::boca::Session session;
EXPECT_CALL(*boca_app_client(), GetSessionManager())
.WillOnce(Return(session_manager()));
EXPECT_CALL(*session_manager(), GetCurrentSession())
.WillOnce(Return(&session));

// API callback.
base::test::TestFuture<std::optional<mojom::UpdateSessionError>> future_1;

boca_app_handler_->EndSession(future_1.GetCallback());
ASSERT_TRUE(future_1.Wait());
EXPECT_EQ(mojom::UpdateSessionError::kInvalid, future_1.Get().value());
}

TEST_F(BocaAppPageHandlerTest, UpdateOnTaskConfigSucceed) {
auto session = GetCommonActiveSessionProto();

Expand Down Expand Up @@ -742,6 +757,22 @@ TEST_F(BocaAppPageHandlerTest, UpdateOnTaskConfigWithEmptySession) {
EXPECT_EQ(mojom::UpdateSessionError::kInvalid, future_1.Get().value());
}

TEST_F(BocaAppPageHandlerTest, UpdateOnTaskConfigWithNonActiveSession) {
::boca::Session non_active_session;
EXPECT_CALL(*boca_app_client(), GetSessionManager())
.WillOnce(Return(session_manager()));
EXPECT_CALL(*session_manager(), GetCurrentSession())
.WillOnce(Return(&non_active_session));

// API callback.
base::test::TestFuture<std::optional<mojom::UpdateSessionError>> future_1;

boca_app_handler_->UpdateOnTaskConfig(GetCommonTestLockOnTaskConfig(),
future_1.GetCallback());
ASSERT_TRUE(future_1.Wait());
EXPECT_EQ(mojom::UpdateSessionError::kInvalid, future_1.Get().value());
}

TEST_F(BocaAppPageHandlerTest, UpdateOnTaskConfigWithHTTPFailure) {
auto session = GetCommonActiveSessionProto();
EXPECT_CALL(*session_manager(), GetCurrentSession())
Expand Down Expand Up @@ -794,6 +825,23 @@ TEST_F(BocaAppPageHandlerTest, UpdateCaptionWithEmptySession) {
EXPECT_EQ(mojom::UpdateSessionError::kInvalid, future_1.Get().value());
}

TEST_F(BocaAppPageHandlerTest, UpdateCaptionWithNonActiveSession) {
::boca::Session session;
EXPECT_CALL(*boca_app_client(), GetSessionManager())
.Times(2)
.WillRepeatedly(Return(session_manager()));
EXPECT_CALL(*session_manager(), GetCurrentSession())
.WillOnce(Return(&session));
EXPECT_CALL(*session_manager(), NotifyLocalCaptionEvents(_)).Times(1);
// API callback.
base::test::TestFuture<std::optional<mojom::UpdateSessionError>> future_1;

boca_app_handler_->UpdateCaptionConfig(GetCommonCaptionConfig(),
future_1.GetCallback());
ASSERT_TRUE(future_1.Wait());
EXPECT_EQ(mojom::UpdateSessionError::kInvalid, future_1.Get().value());
}

TEST_F(BocaAppPageHandlerTest, UpdateCaptionConfigSucceed) {
auto session = GetCommonActiveSessionProto();
EXPECT_CALL(*boca_app_client(), GetSessionManager())
Expand Down
13 changes: 13 additions & 0 deletions ash/webui/boca_ui/resources/app/boca_app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,19 @@ export declare interface ClientApiDelegate {
* Retrivies the current session.
*/
getSession(): Promise<Session|null>;
/**
* End the current session
*/
endSession(): Promise<boolean>;
/**
* Update on task config
*/
updateOnTaskConfig(onTaskConfig: OnTaskConfig): Promise<boolean>;

/**
* Update caption config
*/
updateCaptionConfig(captionConfig: CaptionConfig): Promise<boolean>;
}

/**
Expand Down
41 changes: 31 additions & 10 deletions ash/webui/boca_ui/resources/app/client_delegate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import {Config, ControlledTab as ControlledTabMojom, Course, Identity, PageHandlerRemote, TabInfo, Window} from '../mojom/boca.mojom-webui.js';

import {ClientApiDelegate, ControlledTab, SessionConfig} from './boca_app.js';
import {CaptionConfig, ClientApiDelegate, ControlledTab, OnTaskConfig, SessionConfig} from './boca_app.js';

const MICRO_SECS_IN_MINUTES: bigint = 60000000n;

Expand Down Expand Up @@ -53,17 +53,10 @@ export class ClientDelegateFactory {
});
},
createSession: async (sessionConfig: SessionConfig) => {
console.log(
'Actual session duration is:' +
sessionConfig.sessionDurationInMinutes +
', but we overrided it to 2min');
const result = await pageHandler.createSession({
sessionDuration: {
// TODO(b/365141108) Hardcode session duration to 2 mintes until we
// have end session.
microseconds: BigInt(2) * MICRO_SECS_IN_MINUTES,
// microseconds: BigInt(sessionConfig.sessionDurationInMinutes) *
// MICRO_SECS_IN_MINUTES,
microseconds: BigInt(sessionConfig.sessionDurationInMinutes) *
MICRO_SECS_IN_MINUTES,
},
students: sessionConfig.students,
onTaskConfig: {
Expand Down Expand Up @@ -116,6 +109,34 @@ export class ClientDelegateFactory {
activity: [],
};
},
endSession: async () => {
const result = await pageHandler.endSession();
return !result.error;
},
updateOnTaskConfig: async (onTaskConfig: OnTaskConfig) => {
const result = await pageHandler.updateOnTaskConfig(
{
isLocked: onTaskConfig.isLocked,
tabs: onTaskConfig.tabs.map((item: ControlledTab) => {
return {
tab: {
url: {url: item.tab.url},
title: item.tab.title,
favicon: item.tab.favicon,
},
navigationType: item.navigationType.valueOf(),
};
}),
},
);
return !result.error;
},
updateCaptionConfig: async (captionConfig: CaptionConfig) => {
const result = await pageHandler.updateCaptionConfig(
captionConfig,
);
return !result.error;
},
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// found in the LICENSE file.

import {ClientDelegateFactory} from 'chrome-untrusted://boca-app/app/client_delegate.js';
import {Config, Course, Identity, PageHandlerRemote, SessionResult, Window} from 'chrome-untrusted://boca-app/mojom/boca.mojom-webui.js';
import {CaptionConfig, Config, Course, Identity, OnTaskConfig, PageHandlerRemote, SessionResult, UpdateSessionError, Window} from 'chrome-untrusted://boca-app/mojom/boca.mojom-webui.js';
import {Url} from 'chrome-untrusted://resources/mojo/url/mojom/url.mojom-webui.js';
import {assertDeepEquals, assertTrue} from 'chrome-untrusted://webui-test/chai_assert.js';

Expand Down Expand Up @@ -48,8 +48,7 @@ class MockRemoteHandler extends PageHandlerRemote {
{
sessionDuration: {
// BigInt serialized as string.
// TODO(b/365141108) Fix this after we remove hard-coded duration.
microseconds: 120000000n,
microseconds: 7200000000n,
},
students: [
{id: '1', name: 'cat', email: '[email protected]'},
Expand Down Expand Up @@ -128,6 +127,50 @@ class MockRemoteHandler extends PageHandlerRemote {
},
});
}

override updateOnTaskConfig(config: OnTaskConfig):
Promise<{error: UpdateSessionError | null}> {
assertDeepEquals(
{
isLocked: true,
tabs: [
{
tab: {
url: {url: 'http://google.com/'},
title: 'google',
favicon: 'data/image',
},
navigationType: 0,
},
{
tab: {
url: {url: 'http://youtube.com/'},
title: 'youtube',
favicon: 'data/image',
},
navigationType: 1,
},
],
},
config);
return Promise.resolve({error: null});
}

override updateCaptionConfig(config: CaptionConfig):
Promise<{error: UpdateSessionError | null}> {
assertDeepEquals(
{
captionEnabled: true,
transcriptionEnabled: true,
local: true,
},
config);
return Promise.resolve({error: null});
}

override endSession(): Promise<{error: UpdateSessionError | null}> {
return Promise.resolve({error: null});
}
}

suite('ClientDelegateTest', function() {
Expand Down Expand Up @@ -274,4 +317,46 @@ suite('ClientDelegateTest', function() {
result);
});

test(
'client delegate should translate data for update on task config',
async () => {
const result =
await clientDelegateImpl.getInstance().updateOnTaskConfig({
isLocked: true,
tabs: [
{
tab: {
title: 'google',
url: 'http://google.com/',
favicon: 'data/image',
},
navigationType: 0,
},
{
tab: {
title: 'youtube',
url: 'http://youtube.com/',
favicon: 'data/image',
},
navigationType: 1,
},
],
});
assertTrue(result);
});

test('client delegate should translate data for caption config', async () => {
const result = await clientDelegateImpl.getInstance().updateCaptionConfig({
captionEnabled: true,
local: true,
transcriptionEnabled: true,
});
assertTrue(result);
});

test('client delegate should translate data for end session', async () => {
const result = await clientDelegateImpl.getInstance().endSession();
assertTrue(result);
});

});
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ bool UpdateSessionRequest::GetContentData(std::string* upload_content_type,
*upload_content_type = boca::kContentTypeApplicationJson;

base::Value::Dict root;

// Mandatory data for all update request.
root.Set(kSessionId, session_id_);
base::Value::Dict teacher;
teacher.Set(kGaiaId, teacher_.gaia_id());
root.Set(kTeacher, std::move(teacher));

if (duration_) {
base::Value::Dict duration;
duration.Set(kSeconds, static_cast<int>(duration_->InSeconds()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ TEST_F(UpdateSessionTest, UpdateSessionWithEndSessionInputAndSucceed) {
EXPECT_EQ("/v1/teachers/1/sessions/sessionId?updateMask=sessionState",
http_request.relative_url);
EXPECT_EQ("application/json", http_request.headers["Content-Type"]);
auto* contentData = "{\"sessionState\":3}";
auto* contentData =
"{\"sessionId\":\"sessionId\",\"sessionState\":3,\"teacher\":{\"gaiaId\":"
"\"1\"}}";
ASSERT_TRUE(http_request.has_content);
EXPECT_EQ(contentData, http_request.content);
EXPECT_TRUE(result.has_value());
Expand Down Expand Up @@ -192,7 +194,9 @@ TEST_F(UpdateSessionTest, UpdateSessionWithExendSessionInputAndSucceed) {
EXPECT_EQ("/v1/teachers/1/sessions/sessionId?updateMask=duration",
http_request.relative_url);
EXPECT_EQ("application/json", http_request.headers["Content-Type"]);
auto* contentData = "{\"duration\":{\"seconds\":120}}";
auto* contentData =
"{\"duration\":{\"seconds\":120},\"sessionId\":\"sessionId\",\"teacher\":"
"{\"gaiaId\":\"1\"}}";
ASSERT_TRUE(http_request.has_content);
EXPECT_EQ(contentData, http_request.content);
EXPECT_TRUE(result.has_value());
Expand Down Expand Up @@ -228,7 +232,9 @@ TEST_F(UpdateSessionTest,
"/v1/teachers/1/sessions/sessionId?updateMask=sessionState,duration",
http_request.relative_url);
EXPECT_EQ("application/json", http_request.headers["Content-Type"]);
auto* contentData = "{\"duration\":{\"seconds\":60},\"sessionState\":3}";
auto* contentData =
"{\"duration\":{\"seconds\":60},\"sessionId\":\"sessionId\","
"\"sessionState\":3,\"teacher\":{\"gaiaId\":\"1\"}}";
ASSERT_TRUE(http_request.has_content);
EXPECT_EQ(contentData, http_request.content);
EXPECT_TRUE(result.has_value());
Expand Down Expand Up @@ -291,15 +297,15 @@ TEST_F(UpdateSessionTest, UpdateSessionWithUpdateSessionConfigAndSucceed) {
http_request.relative_url);
EXPECT_EQ("application/json", http_request.headers["Content-Type"]);
auto* contentData =
"{\"studentGroupConfigs\":"
"{\"main\":{\"captionsConfig\":{\"captionsEnabled\":true,"
"\"translationsEnabled\":true},\"onTaskConfig\":{\"activeBundle\":{"
"\"contentConfigs\":[{\"faviconUrl\":\"data:image/"
"{\"sessionId\":\"sessionId\",\"studentGroupConfigs\":{\"main\":{"
"\"captionsConfig\":{\"captionsEnabled\":true,\"translationsEnabled\":"
"true},\"onTaskConfig\":{\"activeBundle\":{\"contentConfigs\":[{"
"\"faviconUrl\":\"data:image/"
"123\",\"lockedNavigationOptions\":{\"navigationType\":1},\"title\":"
"\"google\",\"url\":\"https://google.com\"},{\"faviconUrl\":\"data:image/"
"123\",\"lockedNavigationOptions\":{\"navigationType\":2},\"title\":"
"\"youtube\",\"url\":\"https://"
"youtube.com\"}],\"locked\":true}}}}}";
"youtube.com\"}],\"locked\":true}}}},\"teacher\":{\"gaiaId\":\"1\"}}";
ASSERT_TRUE(http_request.has_content);
EXPECT_EQ(contentData, http_request.content);
EXPECT_TRUE(result.value());
Expand Down Expand Up @@ -331,7 +337,9 @@ TEST_F(UpdateSessionTest, UpdateSessionWithDurationAndFail) {
EXPECT_EQ("/v1/teachers/1/sessions/sessionId?updateMask=duration",
http_request.relative_url);
EXPECT_EQ("application/json", http_request.headers["Content-Type"]);
auto* contentData = "{\"duration\":{\"seconds\":60}}";
auto* contentData =
"{\"duration\":{\"seconds\":60},\"sessionId\":\"sessionId\",\"teacher\":{"
"\"gaiaId\":\"1\"}}";
ASSERT_TRUE(http_request.has_content);
EXPECT_EQ(contentData, http_request.content);
EXPECT_EQ(google_apis::HTTP_INTERNAL_SERVER_ERROR, result.error());
Expand Down

0 comments on commit 98f652c

Please sign in to comment.