-
Notifications
You must be signed in to change notification settings - Fork 492
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
Save creation time of message in ChatMessage
#12570
base: master
Are you sure you want to change the base?
Conversation
WalletWasabi.Fluent/ViewModels/Wallets/Buy/Messages/MessageViewModel.cs
Outdated
Show resolved
Hide resolved
WalletWasabi.Fluent/ViewModels/Wallets/Buy/Messages/UserMessageViewModel.cs
Show resolved
Hide resolved
var timestamp = GetTimestampFromMessage(chatMessage); | ||
|
||
newMessage = newMessage with { Text = timestamp + userMessage }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var timestamp = GetTimestampFromMessage(chatMessage); | |
newMessage = newMessage with { Text = timestamp + userMessage }; | |
newMessage = newMessage with { Text = GetTimestampFromMessage(chatMessage) + userMessage }; |
Since we don't do any checks on timestamp
, maybe we could simplify this. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, but I like my version slightly better for readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late comment, taking a look at the problem now I think it should be the following (Although this is just my opinion, a final decision is @molnard's):
- Whenever we receive the raw string from SIB, during parsing it should also parse the time stamps and store it in ChatMessage's property, and store the message text without the
@xxxxxx@
part. - The UI should not deal with parsing (For the release it was fine, but now we could do it properly) that is the business side's responsibility.
- The UI, when adds a new message, should just set the timestamp property in ChatMessage.
- When we send back the whole conversation, the business side should form it properly back to a raw string (get the timestamp from the property and paste in front of the message text in the correct format).
Partially fixes #12429
This is only business code. The time of creation now gets saved in the ChatMessage object, for both user and agent messages, but it's not visible anywhere, that will be a UI task. @zkSNACKs/ui-team
What happened:
ParseRawMessage
fromAssistantViewModel
and moved it to the base,MessageViewModel
.UiMessage
, as now it's handled in the base class.OfferMessageViewModel
, where it has to be overwritten.WorkflowStep.EditMessageAsync
so the timestamp from the raw text doesn't disappear.ChatMessage
record, so it readsDateTime
from the Unix timestamp of the message.