-
Notifications
You must be signed in to change notification settings - Fork 89
Update profile #561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Update profile #561
Changes from all commits
c93d074
5eecd71
64f0643
0ef1d42
ad7aabd
4117588
0fdd80f
c854c71
2b9374c
047b7f9
5b8e4f8
556f74d
83d087a
92623a2
1e2aaa2
efa8155
7219595
3aaad2e
d5a81b2
838ac3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| <!-- | ||
| SPDX-FileCopyrightText: 2025 The BAR Lobby Authors | ||
|
|
||
| SPDX-License-Identifier: MIT | ||
| --> | ||
|
|
||
| <template> | ||
| <div class="message" :class="[{ 'from-host': fromHost }, message.type]"> | ||
| <div v-if="user" class="user"> | ||
| <Flag :countryCode="user.countryCode" style="width: 16px" /> | ||
| <div>{{ user.username }}</div> | ||
| </div> | ||
| <Markdown :source="message.text" /> | ||
| </div> | ||
| </template> | ||
|
|
||
| <script lang="ts" setup> | ||
| import { User } from "@main/model/user"; | ||
| import Flag from "@renderer/components/misc/Flag.vue"; | ||
| import Markdown from "@renderer/components/misc/Markdown.vue"; | ||
| import { Message } from "@renderer/model/messages"; | ||
| import { me } from "@renderer/store/me.store"; | ||
|
|
||
| defineProps<{ | ||
| message: Message; | ||
| }>(); | ||
|
|
||
| const user: User = { | ||
| battleRoomState: {}, | ||
| countryCode: "US", | ||
| userId: "123", | ||
| clanBaseData: null, | ||
| username: "Test User", | ||
| displayName: "Test User", | ||
| partyId: "123", | ||
| status: "lobby", | ||
| isMe: false, | ||
| }; | ||
|
|
||
| // const user = api.session.getUserById(props.message.senderUserId); | ||
| const fromHost = user.userId === me.userId; | ||
| </script> | ||
|
|
||
| <style lang="scss" scoped> | ||
| .message { | ||
| display: flex; | ||
| flex-direction: row; | ||
| background: rgba(255, 255, 255, 0.1); | ||
| border: 1px solid rgba(255, 255, 255, 0.1); | ||
| border-radius: 3px; | ||
| overflow: hidden; | ||
| &.from-host { | ||
| background: rgba(110, 186, 216, 0.4); | ||
| } | ||
| &.battle-announcement { | ||
| background: rgba(119, 255, 180, 0.24); | ||
| } | ||
| } | ||
| .user { | ||
| display: flex; | ||
| flex-direction: row; | ||
| align-items: center; | ||
| gap: 5px; | ||
| padding: 4px 8px; | ||
| background: rgba(255, 255, 255, 0.05); | ||
| border-right: 1px solid rgba(255, 255, 255, 0.1); | ||
| box-shadow: 2px 0 5px rgba(0, 0, 0, 0.4); | ||
| font-weight: 500; | ||
| } | ||
| </style> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ | |
| import { db } from "@renderer/store/db"; | ||
| import { reactive } from "vue"; | ||
| import { SubsManager } from "@renderer/utils/subscriptions-manager"; | ||
| import { UserInfoOkResponseData } from "tachyon-protocol/types"; | ||
| import { apply as applyPatch } from "json8-merge-patch"; | ||
|
|
||
| export const usersStore: { | ||
| isInitialized: boolean; | ||
|
|
@@ -23,24 +25,54 @@ export function initUsersStore() { | |
| console.warn("Received user/updated event with no userId, skipping update."); | ||
| return; | ||
| } | ||
| const updated = await db.users.update(user.userId, { ...user }); | ||
|
|
||
|
Rimek86 marked this conversation as resolved.
|
||
| const existingUser = await db.users.get(user.userId); | ||
| const updatedUser = applyPatch(existingUser || {}, { | ||
| ...user, | ||
| clanBaseData: user.clanBaseData | ||
| ? { | ||
| ...user.clanBaseData, | ||
| language: user.clanBaseData.language || "unknown", | ||
| } | ||
| : undefined, | ||
| }); | ||
|
|
||
| const updated = await db.users.update(user.userId, updatedUser); | ||
|
|
||
| if (updated === 0) { | ||
| // No records updated, so user doesn't exist - create new user | ||
| db.users.add({ | ||
| userId: user.userId, | ||
| username: user.username ?? "Unknown User", | ||
| displayName: user.displayName ?? "Unknown User", | ||
|
Comment on lines
+46
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there's a problem there. If you don't have anything in the DB, but you are getting a partial update, then you can't really create the correct record. So either you need to check that the event you got has all the data, that is, it's the initial event and use that. Or, if it's a partial update, you can't use the event and would need to I am not sure how that could happen, but creating a user with these default seems just wrong.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The original code just assumes that it can happen and fills in some blanks, no harm done if server functions correctly, but we can make it cleaner.
Not really. Recall the issues with ordering of events/reponses. This would happen here too.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the original author programmed defensively here. I don't think it's wrong to do it that way. It only covers the case where there is inconsistent or incomplete data. It should never happen... @p2004a How we can make it cleaner? (My goal wasn't to imrpove the users.store.ts My goal was to improve the profiles page here. :-) ) You once asked me not to change too much at once. That's why I wanted to stick to it and not touch everything directly with a PR. Can we possibly make this an issue that will then be changed later?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @p2004a Hi, i was absent in the last weeks. But now i thin i find time again to continue. What's about my question so that i can continoue here...
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It needs to be changed in protocol to be merge patch too, including all the implications of that: drop all the allowed nulls, it's reserved to patch semantic. (overall, having a optional field that can also take null is weird)
I mean, piling up more mess on the top is not correct strategy, this is not deep refactoring, you are changing this code. All this code can be simplified to const toUpdate = await db.users.bulkGet(event.users.map((u) => u.userId));
const updatedUsers = event.users.map((u, i) => applyPatch(toUpdate[i] || {}, u));
await db.users.bulkPut(updatedUsers); |
||
| clanId: null, | ||
| clanBaseData: user.clanBaseData | ||
| ? { | ||
| ...user.clanBaseData, | ||
| language: user.clanBaseData.language || "unknown", | ||
| } | ||
| : null, | ||
| partyId: null, | ||
| countryCode: "??", | ||
| status: "offline", | ||
| countryCode: user.countryCode ?? "??", | ||
| status: user.status ?? "offline", | ||
| roles: user.roles ?? [], | ||
| rating: user.rating ?? null, | ||
| battleRoomState: {}, | ||
| ...user, // Override defaults with actual data | ||
| isMe: false, | ||
| ...user, | ||
| }); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| usersStore.isInitialized = true; | ||
| } | ||
|
|
||
| export async function fetchUserInfo(userId: string): Promise<UserInfoOkResponseData | null> { | ||
| try { | ||
| const response = await window.tachyon.request("user/info", { userId: userId }); | ||
| return response.data; | ||
| } catch (error) { | ||
| console.error("Error fetching user info:", error); | ||
| return null; | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.