tarsil / django-messages-drf

Simple messaging system for Django implementing Django Rest Framework
https://tarsil.github.io/django-messages-drf/
MIT License
22 stars 6 forks source link

Sentbox functionality - upvote if needed #19

Open hvitis opened 1 year ago

hvitis commented 1 year ago

Hi, thanks for a wonderful package!

I have a question regarding the logic:

Case:

User A initiates a new thread by sending a first message to user B

Current behaviour:

There is no way for user A to see the message that has just been sent by using API. Sent UUID has to be saved on front-end in order to call GET ThreadByUUID endpoint. By calling GET /inbox url only user B (msg recipient) sees a new thread.

Expected behaviour:

When I am sending a new message to someone, I would like to have it visible somewhere - endpoint "sent or initiated" perhaps ?

What do you think?

tarsil commented 1 year ago

@hvitis actually that is a good question. The logic is actually there. You are asking for a sent box :).

I actually have that in my personal projects and not here and that was intentional because a lot of projects using this do not want to have sent boxes but the logic can be added easily.

If you subclass the ThreadModel you can see the inbox logic, there you can see the flags being used to do that logic. For the sent, you can simply create another function and revert the logic πŸ™‚.

I understand where you are coming from, truly πŸ™‚.

With that, you can simply override the view that handles with that and call your sent box with the same serializer (if you inherit). This would be the quickest and cleanest way without impacting anything.

I'm also opened to contributions if you think you can add that special feature and if is required. It wouldn't hurt anyway.

I'm super looking forward to have people joining my Opensource projects and progressing them as well πŸš€πŸš€

tarsil commented 1 year ago

Also, I don't do aot of release because the package is somewhat stable but can always evolve.

I'm always on it for maintenance of every Opensource project I build.

hvitis commented 1 year ago

What a superb contact and truly professional approach.

I also think that it's very well written and customizable. I am currently extending existing classes to implement notifications and surely making sentbox is not a problem. I was just wondering if that would be something worth adding to the package.

Since you're saying that some people prefer not to have it I will leave this thread hanging for input from others (upvote it if you need a sentbox). If there's a need I would be more than happy to help.

Again, thank you for a great app and let me know if you need anything.

Best regards

tarsil commented 1 year ago

@hvitis you truly made my day and I'm always more than happy to help and explain why some decisions were made by that time.

It is important to remember though, that was a couple of years ago and needs also change.

I would be more than happy for you to create a pull request with the changes you would like to add because I might be missing something out as well and worse case scenario, people just don't use that piece of functionality.

I'm more than happy to have your contributions in the project and merge them πŸ˜„

Let me know if you need anything else.

P.S.: For notifications or live notifications some people use django signals for it. I personally with django prefer an API that checks every X minutes and caches it for 120s but it's up to you :)

If you like my work, you should definitely have a look at some projects that I built and actively maintain.

https://esmerald.dev - If you like Django, you will absolutely love this, trust me. I only use this for basically everything I do (besides django). New monthly release coming next week with a lot of even greater stuff.

https://saffier.tarsild.io - An async ORM on the top of SQLALCHEMY but again, with a very familiar interface and I'm sure you will love it too. New release hopefully coming out tomorrow or Friday.

https://asyncz.tarsild.io - What if APScheduler was more modern and fully async? That.

https://databasez.tarsild.io - Making your async database connections simpler.

Best regards

hvitis commented 1 year ago

@tarsil That's an impressive amount of open source work! Chapeaux bas! 🎩

I think I have a couple of suggestions as I must say, at the beginning I was fighting a bit with a couple of issues.

  1. The documentation would be much more helpful if the format and description (e.g. URLs) had some examples or other formatting. (surely something I can help with but maybe with a new doc template? This one looks great.)
  2. The logic with creating a thread and replying to a thread seems to be broken. New thread is being created with both URLs (one with thread UUID/withour a user and one with only the user id) The code comment that describes that functionality:
    If a UUID is passed, then a Thread is validated and created but if only a user_id is
    passed, then it will create a new thread and start a conversation.
  3. The sentbox (we have talked about it already)
  4. Limiting amount of threads - currently I can spam any amount of threads to any user. I think default should be 1 thread per user.
  5. Would be great if the API was following convention (mostly regarding using nouns and plural verb forms. I think nesting also could be improved, especially when the thread/msg relation is involved)

Let me know which point I could cover with some PR. I think this is the only drf messaging system (therefore should be the best πŸ˜€)

tarsil commented 1 year ago

@hvitis thank you very much for the kind comment regarding the Opensource πŸ™‚.

I'll confess, Django messages DRF is probably not my best documentation if comparing with those other Opensource tools, I think. Maybe we refactor the whole thing might be something to consider.

Considering your points, I believe everything can be converted in PRs actually ☺️. We should probably also update to the latest Django and Django DRF to make sure it's covered as well.

Limiting the amount of threads might be something to think in the future. Reason? Ok, imagine your Gmail inbox or any email inbox or even LinkedIn inbox. A thread it will be like an email. If you limit to 1, means you can only send one message to one user, ever. After that done, if you want to send to another user, you won't be able. Does this make sense?

If you like this package maybe I can add you as one of the maintainers as this is a great package to have :)

hvitis commented 1 year ago

I am glad for the invitation to contributors but I did not contribute anything (yet). I will try to suggest PRs in the near future.

You're right regarding the multiple-threads functionality. It indeed can work like that. In this case, sending a new thread is just like sending a message and replying to this message would be creating a thread (other way around from semantics point of view).

I think that in this case there is no need of changing everything and twisting it, I think that for now this issue could be solved just by adding more to documentation. I will try changing it and will open a new issue FYI.