fix: don't remove timeouts when a member leaves a server #409
Conversation
There was a problem hiding this comment.
Solution lgtm!
Could I ask for at least one test that ensures the database model is handled correctly? Specifically, >create member, >add timeout, >kick member, >create member, (check timeout still exists). Thinking about potentially catching issues if we migrate between databases and this logic is lost.
No need to do testing with rocket, a small database test next to the model would suffice. Example use of test harness if you haven't used database_test! yet: https://github.com/revoltchat/backend/blob/main/crates/core/database/src/models/bots/model.rs#L170
| /// Insert a new server member into the database | ||
| async fn insert_member(&self, member: &Member) -> Result<()> { |
There was a problem hiding this comment.
For clarity, I think we should change the comment / rename the function here.
Specifically, something such as insert_or_merge_member, with the comment Insert a new server member (or use the existing member if one is found)
| let member = server_members.get_mut(id); | ||
| if let Some(member) = member { | ||
| if member.in_timeout() { | ||
| panic!("Soft deletion is not implemented.") |
There was a problem hiding this comment.
Impl not possible without modifying the member object since the reference db uses members directly.
| let mut server_members = self.server_members.lock().await; | ||
| if server_members.remove(id).is_some() { | ||
| Ok(()) | ||
| } else { | ||
| Err(create_error!(NotFound)) | ||
| } | ||
| } | ||
|
|
||
| async fn remove_dangling_members(&self) -> Result<()> { | ||
| todo!() |
There was a problem hiding this comment.
Impl not possible without modifying the member object since the reference db uses members directly.
github-project-automation
bot
moved this from 💬 Awaiting review
to 🛑 Changes requested
in Pull Request Overview
Members with timeouts are now persisted until they rejoin or the timeout expires. Members who have left do not show up in API payloads.
Please make sure to check the following tasks before opening and submitting a PR