boca: Fix heartbeat status not reflected.
Heartbeat only update device status not student status. We should parse accordingly. Also change to only return single device. Since UI only support single device now, make chromium side aligned to make the behavior deterministic. Bug: b:403357101 Test: Unit tested Change-Id: If28c47261c47ea45538bf2565da3108997fdfbb1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6355801 Reviewed-by: Benjamin Zielinski <bzielinski@google.com> Commit-Queue: April Zhou <aprilzhou@google.com> Cr-Commit-Position: refs/heads/main@{#1433662}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
f4e3036b23
commit
f190ffc236
ash/webui/boca_ui
chromeos/ash/components/boca/session_api
@ -162,17 +162,22 @@ std::vector<mojom::IdentifiedActivityPtr> SessionActivityProtoToMojom(
|
||||
const std::map<std::string, ::boca::StudentStatus>& activities) {
|
||||
std::vector<mojom::IdentifiedActivityPtr> result;
|
||||
for (auto item : activities) {
|
||||
for (auto device : item.second.devices()) {
|
||||
// Only update state and active tab now.
|
||||
if (auto const device = item.second.devices().begin();
|
||||
device != item.second.devices().end()) {
|
||||
// Only update state and active tab for the first device now.
|
||||
// TODO - crbug.com/403655119: Ideally we should support multi-device. But
|
||||
// since now UI only supports single device, always parse the first one to
|
||||
// make the behavior deterministic.
|
||||
auto identity_ptr = mojom::IdentifiedActivity::New(
|
||||
item.first, mojom::StudentActivity::New(
|
||||
item.second.state() == ::boca::StudentStatus::ACTIVE,
|
||||
device.second.activity().active_tab().title(),
|
||||
/*is_caption_enabled=*/false,
|
||||
/*is_hand_raised=*/false, mojom::JoinMethod::kRoster,
|
||||
device.second.view_screen_config()
|
||||
.connection_param()
|
||||
.connection_code()));
|
||||
item.first,
|
||||
mojom::StudentActivity::New(
|
||||
device->second.state() == ::boca::StudentDevice::ACTIVE,
|
||||
device->second.activity().active_tab().title(),
|
||||
/*is_caption_enabled=*/false,
|
||||
/*is_hand_raised=*/false, mojom::JoinMethod::kRoster,
|
||||
device->second.view_screen_config()
|
||||
.connection_param()
|
||||
.connection_code()));
|
||||
result.push_back(std::move(identity_ptr));
|
||||
}
|
||||
}
|
||||
|
@ -730,14 +730,10 @@ TEST_F(BocaAppPageHandlerTest, GetSessionWithFullInputTest) {
|
||||
auto* activity_1 = device_1.mutable_activity();
|
||||
activity_1->mutable_active_tab()->set_title("google");
|
||||
::boca::StudentDevice device_11;
|
||||
auto* activity_11 = device_11.mutable_activity();
|
||||
activity_11->mutable_active_tab()->set_title("google1");
|
||||
device_11.mutable_view_screen_config()
|
||||
->mutable_connection_param()
|
||||
->set_connection_code("abcd");
|
||||
device_1.set_state(::boca::StudentDevice::INACTIVE);
|
||||
|
||||
(*status_1.mutable_devices())["device1"] = std::move(device_1);
|
||||
(*status_1.mutable_devices())["device11"] = std::move(device_11);
|
||||
(*student_statuses)["000"] = std::move(status_1);
|
||||
(*student_statuses)["111"] = std::move(status_1);
|
||||
|
||||
request->callback().Run(std::move(session));
|
||||
})));
|
||||
@ -786,7 +782,9 @@ TEST_F(BocaAppPageHandlerTest, GetSessionWithFullInputTest) {
|
||||
EXPECT_EQ("http://data/image", result->on_task_config->tabs[0]->tab->favicon);
|
||||
|
||||
auto activities = std::move(result0->activities);
|
||||
EXPECT_EQ(2u, activities.size());
|
||||
EXPECT_EQ(1u, activities.size());
|
||||
EXPECT_FALSE(activities[0]->activity->is_active);
|
||||
EXPECT_EQ("google", activities[0]->activity->active_tab);
|
||||
}
|
||||
|
||||
TEST_F(BocaAppPageHandlerTest, GetSessionWithPartialInputTest) {
|
||||
@ -1570,25 +1568,23 @@ TEST_F(BocaAppPageHandlerTest, UpdateEmptyStudentActivitySucceed) {
|
||||
ASSERT_TRUE(result.empty());
|
||||
}
|
||||
|
||||
TEST_F(BocaAppPageHandlerTest, DISABLED_UpdateNonEmptyStudentActivitySucceed) {
|
||||
TEST_F(BocaAppPageHandlerTest, UpdateNonEmptyStudentActivitySucceed) {
|
||||
std::map<std::string, ::boca::StudentStatus> activities;
|
||||
::boca::StudentStatus status_1;
|
||||
status_1.set_state(::boca::StudentStatus::ACTIVE);
|
||||
::boca::StudentDevice device_1;
|
||||
auto* activity_1 = device_1.mutable_activity();
|
||||
activity_1->mutable_active_tab()->set_title("google");
|
||||
::boca::StudentDevice device_11;
|
||||
auto* activity_11 = device_11.mutable_activity();
|
||||
activity_11->mutable_active_tab()->set_title("google1");
|
||||
device_11.mutable_view_screen_config()
|
||||
device_1.set_state(::boca::StudentDevice::INACTIVE);
|
||||
device_1.mutable_view_screen_config()
|
||||
->mutable_connection_param()
|
||||
->set_connection_code("abcd");
|
||||
(*status_1.mutable_devices())["device1"] = std::move(device_1);
|
||||
(*status_1.mutable_devices())["device11"] = std::move(device_11);
|
||||
|
||||
::boca::StudentStatus status_2;
|
||||
status_2.set_state(::boca::StudentStatus::ADDED);
|
||||
::boca::StudentDevice device_2;
|
||||
device_2.set_state(::boca::StudentDevice::ACTIVE);
|
||||
auto* activity_2 = device_2.mutable_activity();
|
||||
activity_2->mutable_active_tab()->set_title("youtube");
|
||||
(*status_2.mutable_devices())["device2"] = std::move(device_2);
|
||||
@ -1600,28 +1596,17 @@ TEST_F(BocaAppPageHandlerTest, DISABLED_UpdateNonEmptyStudentActivitySucceed) {
|
||||
fake_page()->SetActivityInterceptorCallback(future.GetCallback());
|
||||
boca_app_handler()->OnConsumerActivityUpdated(activities);
|
||||
auto result = future.Take();
|
||||
EXPECT_EQ(3u, result.size());
|
||||
// Verify multiple devices are both added.
|
||||
EXPECT_EQ(2u, result.size());
|
||||
// Verify only first device added.
|
||||
EXPECT_EQ("1", result[0]->id);
|
||||
EXPECT_TRUE(result[0]->activity->is_active);
|
||||
EXPECT_EQ("1", result[1]->id);
|
||||
EXPECT_TRUE(result[1]->activity->is_active);
|
||||
|
||||
std::vector<std::string> tabs = {"google", "google1"};
|
||||
// The order shouldn't matter.
|
||||
EXPECT_TRUE(std::find(tabs.begin(), tabs.end(),
|
||||
result[0]->activity->active_tab) != tabs.end());
|
||||
EXPECT_TRUE(std::find(tabs.begin(), tabs.end(),
|
||||
result[1]->activity->active_tab) != tabs.end());
|
||||
|
||||
EXPECT_EQ("2", result[2]->id);
|
||||
EXPECT_EQ("youtube", result[2]->activity->active_tab);
|
||||
EXPECT_FALSE(result[2]->activity->is_active);
|
||||
|
||||
EXPECT_FALSE(result[0]->activity->is_active);
|
||||
EXPECT_EQ("google", result[0]->activity->active_tab);
|
||||
// Connection code should be set
|
||||
EXPECT_EQ("abcd", result[0]->activity->view_screen_session_code);
|
||||
EXPECT_EQ("", result[1]->activity->view_screen_session_code);
|
||||
EXPECT_EQ("", result[2]->activity->view_screen_session_code);
|
||||
|
||||
EXPECT_EQ("2", result[1]->id);
|
||||
EXPECT_EQ("youtube", result[1]->activity->active_tab);
|
||||
EXPECT_TRUE(result[1]->activity->is_active);
|
||||
}
|
||||
|
||||
TEST_F(BocaAppPageHandlerTest, RemoveStudentSucceedAlsoRemoveFromLocalSession) {
|
||||
|
@ -71,6 +71,7 @@ inline constexpr char kCaptionsEnabled[] = "captionsEnabled";
|
||||
inline constexpr char kTranslationsEnabled[] = "translationsEnabled";
|
||||
inline constexpr char kStudentStatus[] = "studentStatuses";
|
||||
inline constexpr char kStudentStatusState[] = "state";
|
||||
inline constexpr char kDeviceStatusState[] = "state";
|
||||
inline constexpr char kUrl[] = "url";
|
||||
inline constexpr char kTitle[] = "title";
|
||||
inline constexpr char kFavIcon[] = "faviconUrl";
|
||||
|
@ -33,6 +33,20 @@ namespace ash::boca {
|
||||
return ::boca::StudentStatus::STUDENT_STATE_UNKNOWN;
|
||||
}
|
||||
|
||||
::boca::StudentDevice::StudentDeviceState DeviceStatusJsonToProto(
|
||||
const std::string& status) {
|
||||
if (status == "STUDENT_DEVICE_STATE_UNKNOWN") {
|
||||
return ::boca::StudentDevice::STUDENT_DEVICE_STATE_UNKNOWN;
|
||||
}
|
||||
if (status == "ACTIVE") {
|
||||
return ::boca::StudentDevice::ACTIVE;
|
||||
}
|
||||
if (status == "INACTIVE") {
|
||||
return ::boca::StudentDevice::INACTIVE;
|
||||
}
|
||||
return ::boca::StudentDevice::STUDENT_DEVICE_STATE_UNKNOWN;
|
||||
}
|
||||
|
||||
::boca::Session::SessionState SessionStateJsonToProto(
|
||||
const std::string& state) {
|
||||
if (state == "SESSION_STATE_UNKNOWN") {
|
||||
@ -315,6 +329,9 @@ void ParseIndividualStudentStatusFromJson(
|
||||
if (auto* device_dict = device_iter.second.GetIfDict()) {
|
||||
auto& device_entry =
|
||||
(*student_status->mutable_devices())[device_iter.first];
|
||||
if (auto* state_ptr = device_dict->FindString(kDeviceStatusState)) {
|
||||
device_entry.set_state(DeviceStatusJsonToProto(*state_ptr));
|
||||
}
|
||||
// Parse and set ActiveTab from StudentDeviceActivity
|
||||
if (auto* activity = device_dict->FindDict(kActivity)) {
|
||||
if (auto* active_tab_ptr = activity->FindDict(kActiveTab)) {
|
||||
@ -324,6 +341,7 @@ void ParseIndividualStudentStatusFromJson(
|
||||
: "");
|
||||
}
|
||||
}
|
||||
|
||||
if (::ash::features::IsBocaSpotlightEnabled()) {
|
||||
if (auto* view_screen_config_dict =
|
||||
device_dict->FindDict(kViewScreenConfig)) {
|
||||
|
@ -43,7 +43,7 @@ constexpr char kFullSessionResponse[] = R"(
|
||||
"kDummyDeviceId":
|
||||
{
|
||||
"info": {"device_id":"kDummyDeviceId"},
|
||||
"state":"ACTIVE",
|
||||
"state":"INACTIVE",
|
||||
"activity": {
|
||||
"activeTab": {
|
||||
"title": "google"
|
||||
@ -155,7 +155,7 @@ constexpr char kPartialResponse[] = R"(
|
||||
"kDummyDeviceId":
|
||||
{
|
||||
"info": {"device_id":"kDummyDeviceId"},
|
||||
"state":"ACTIVE",
|
||||
"state":"INACTIVE",
|
||||
"activity": {
|
||||
"activeTab": {
|
||||
"title": "google"
|
||||
@ -398,6 +398,11 @@ TEST_F(SessionParserTest, TestParseStudentStatusProtoFromJson) {
|
||||
.activity()
|
||||
.active_tab()
|
||||
.title());
|
||||
EXPECT_EQ(::boca::StudentDevice::INACTIVE, session_partial->student_statuses()
|
||||
.at("3")
|
||||
.devices()
|
||||
.at("kDummyDeviceId")
|
||||
.state());
|
||||
EXPECT_EQ(::boca::ViewScreenConfig::REQUESTED,
|
||||
session_partial->student_statuses()
|
||||
.at("3")
|
||||
|
Reference in New Issue
Block a user