feat: Chat request implementation #14
No reviewers
Labels
No labels
Good First Issue
Help wanted
Kind/Breaking
Kind/Bug
Kind/Bug Fix
Kind/CI-CD
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Question
Kind/Security
Kind/Testing
Reviewed
Accepted
Reviewed
Confirmed
Reviewed
Declined
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Approved
Status
Blocked
Status
Inactive
Status
Need More Info
Status
Waiting On Author
Status
Waiting On Review
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Blocks
#2 Phase 3: In server communication}
OxideTalis/oxidetalis
Reference: OxideTalis/oxidetalis#14
Loading…
Reference in a new issue
No description provided.
Delete branch "awiteb/chat-request-and-response"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Reslove #2
Chat request implementation of the OTMP protocol
feat: Chat request implementationto WIP: feat: Chat request implementation1356ae40d0
to4abd0241e1
I hate big PR, but there is no way to make it smaller. All thing are related and need each other.
870ffef0c1
to33677cf055
33677cf055
toabed78fd77
abed78fd77
to6744537aab
14967dfd1c
to6f67474b5f
5a5ffb308f
to8392d87593
6e782d1e20
to53db94c38f
WIP: feat: Chat request implementationto feat: Chat request implementationI'll re-base it on the master
7512d1e25f
to1857365aac
@Amjad50 Can you review it? If you can, take your time. If not, no problem, I will review it
haha, yah understandable.
I'll review it soon
Take your time, there is couples
TODO
andFIXME
I'll resolve it in another PR, after merging these@ -0,0 +29,4 @@
/// Extension trait for the `DatabaseConnection` to work with the blacklist
/// table
pub trait BlackListExt {
/// Returns ture if the `blacklister` are blacklisted the
ture
->true
(use find and replace all, there are multiple of these)blacklister has blacklisted
@ -0,0 +62,4 @@
}
impl BlackListExt for DatabaseConnection {
#[logcall::logcall]
I noticed that several methods have
logcall
but others don't, is it remaining from debugging session? or planning to keep itIt was for debugging session, but I will use it on more functions
There are a lot of functions that need to be checked, 300+ functions to check if the log is necessary or not, I hate this
Ok, it is just 100 functions
@ -0,0 +45,4 @@
.filter(InChatRequestsColumn::Sender.eq(sender.to_string()))
.one(self)
.await?
.is_none()
since we are not dealing with the result. maybe a one statement approach to do
here i'm updating
InOn
if needed, but can also be changed todo_nothing()
@ -0,0 +30,4 @@
&self,
requester: &UserModel,
recipient: &PublicKey,
) -> ServerResult<Option<OutChatRequestsModel>>;
should return
true
based on the name and description of the funcsmall nitpick (also applies to
have_chat_request_to
below)I would name it
get_chat_request_to
, ashave
seems that it would be boolean@ -0,0 +29,4 @@
/// Extension trait for the `DatabaseConnection` to work with the whitelist
/// table
pub trait WhiteListExt {
/// Returns ture if the `whitelister` are whitelisted the
ture
The whitelister has whitelisted
@ -113,0 +130,4 @@
}
async fn send(&self, conn_id: &Uuid, event: ServerEvent<Unsigned>) {
if let Some((_, user)) = self.read().await.iter().find(|(c, _)| *c == conn_id) {
use
get
@ -51,0 +56,4 @@
/// New chat request from someone
ChatRequest { from: PublicKey },
/// New chat request response from someone
ChatRequestResponse { from: PublicKey, accepted: bool },
in the client, you mentioned to make it alphabetical, should it be the same here?
Yes, I forgot it
@ -0,0 +50,4 @@
if try_ws!(db.is_blacklisted(&to_user, &from_public_key).await) {
return ServerEvent::message(
"You are unable to send a chat request because you are on the recipient's blacklist.",
Maybe would be better to move this to
WsError
easier to manage errorsthere are also others below this
Right, this should be an error, not a message. I don't know how is this happened
I'll remove
Message
event it is actually useless. I don't know why I thought it was good.Well, I remembered when I added it, it was a pain to add a new error (before
ws_error
macro) so I thought it was good for simple messages.@ -0,0 +18,4 @@
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.
I was thinking we can have one table for blacklist and whitelist.
since at any point in time a user can either be in blacklist or whitelist (or none), instead of doing
it can be a column containing the state, either whitelist or blacklist
@ -0,0 +41,4 @@
)
.col(ColumnDef::new(Blacklist::UserId).big_integer().not_null())
.col(ColumnDef::new(Blacklist::Target).string().not_null())
.col(ColumnDef::new(Blacklist::Reason).string_len(400))
reason is unused
I forgot to remove it, this should be a feature soon. Is good right?
yah, if you want to add it, I think we can do in this PR, just change the whitelist table (or if we are merging both, it will be a special column for blacklist)
So we can either remove it or add it as a feature
Let's make it later, since we'll merge them.
I don't want to think about it right now
@ -0,0 +41,4 @@
use crate::routes::{ApiError, ApiResult};
#[derive(Debug)]
pub struct Pagination {
We can add
EndpointArgRegister
implementation and thus we can use it in the arg directlyIf we go with this, we can also use
ToParameters
macro to generate the implementation.The issue with it is that it doesn't use
ApiError
, but if we are using it as a paremeter,salvo
will handle the error reportingI'll see it, the reason why I don't put it as an argument, is because
Salvo ask me to implement
ToSchema
to it.As I see, Salvo want it as one parameter, but the pagination
is two parameters. I'll see
Ok, I get it. It ask me to implement
ToSchema
because of thisSo I can just implement the
EndpointArgRegister
trait and not usingQueryParam
.Thank you @Amjad50
AlreadyOnTheBlacklist
error name 727b985847Users
enum public a894089b19whitelist
&blacklist
tables withusers_status
table 3d086e511cTableCreateStatement::foreign_key
for foreign keysHashMap::get
instead ofIter::find
ServerEventType::ChatRequestResponse
event alphabeticallyOption
ofServerEvent
in chat request handlers 09f90b060fServerEvent::Message
event 8f791acd5dsea_orm_migration
prelude 02a0ac09ccRecipientId
andSender
9039ee66f0SenderId
andRecipient
9c1ba97791UserId
andTarget
@ -0,0 +103,4 @@
whitelister: &UserModel,
target_public_key: &PublicKey,
) -> ServerResult<bool> {
whitelister
you can use
get_user_status
here and alsois_blacklisted
. also inremove...
functionsEndpointArgRegister
forPagination
d0d048c13511b64caa85
to21880c5f82
OutChatRequestsExt::have_chat_request_to
function toget_chat_request_to
Nice