hyperlog-core / hyperlog-backend

Backend for hyperlog
0 stars 0 forks source link

Add messaging system for outsider to contact hyperlog-user #114

Closed nikochiko closed 3 years ago

nikochiko commented 3 years ago

Changes:

BrainBuzzer commented 3 years ago

This should have filters. If a filter is applied for archived messages, only archieved messages and their count shall be shown.

BrainBuzzer commented 3 years ago

It'll also be a good idea to add a boolean in the query to check if the pagination has next page or not.

nikochiko commented 3 years ago

For filters, do you mean like custom filters as in with JSON for example? (Like messages(filters: {"is_archived": true}) {...}). That can be done, tho I think adding more options in arguments could be more elegant. Or are you saying that we build something like GitHub issues and PR filtering?

For pagination, I had thought I added the number of pages count in it, but it's just the count of message objects. I thought page count could be also used to show number of pages, so do you think that would work or should we keep both?

On Tue, Oct 13, 2020, 16:49 Aditya Giri notifications@github.com wrote:

It'll also be a good idea to add a boolean in the query to check if the pagination has next page or not.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hyperlog/hyperlog-backend/pull/114#issuecomment-707671969, or unsubscribe https://github.com/notifications/unsubscribe-auth/AI7MKYNBB756UAD7ER6V4ALSKQZS3ANCNFSM4SM22KSA .

BrainBuzzer commented 3 years ago

Check out the GitHub GraphQL API. Try to paginate and filter things there in something like user repositories query, to get the idea of what I'm trying to say.

BrainBuzzer commented 3 years ago

Not the custom filters. Just normal filters since we don't have many categories to filter by.

nikochiko commented 3 years ago

Ok so, I think adding arguments like onEachPage, isArchived, orderBy should get the job done. Maybe a sender_name or sender_email could also be added later on. So, I mean something like this:

outsiderMessages(page: Int!, onEachPage: Int, isArchived: Boolean, orderBy: String) {
  messages: [OutsiderMessage]!
  count: Int!
  pages: Int!
}

Or we could do the after, before, first, last format instead of page and onEachPage and the frontend will need to divide it into pages. What do you think?

nikochiko commented 3 years ago

By default, orderBy would be time the message was sent, in a descending order. But I imagine we could set it to other properties like alphabetically by sender name

BrainBuzzer commented 3 years ago

onEachPage is better than the after before style for our use case. And the other features also seem nice.

nikochiko commented 3 years ago

Done. The orderBy argument expects a list of field names to sort according to. By default it sorts according to the rule ["-time"], so it will order the messages by its time field and its descending order (- minus sign before it shows descending). The names should be put in there in decreasing order of priority. Putting sender_name and sender_email in it works. Putting in ? will make it order randomly. (Same as this -> https://docs.djangoproject.com/en/2.2/ref/models/querysets/#order-by).

isArchived works, if nothing is supplied it will give all messages and filter otherwise.

nikochiko commented 3 years ago

We can get creative with filters too, like giving choice of filter expressions totally on frontend (all these -> https://docs.djangoproject.com/en/2.2/ref/models/querysets/#field-lookups) but I guess we could keep it simple if we made easy shortcuts for few combinations of them

BrainBuzzer commented 3 years ago

Remove all restrictions for the phone number from the backend for now. We'll need some better system on the backend to verify the phone number.

The frontend is formatting the phone number in a certain manner, for example, my number is automatically formatted to +91 96373-05012 and that is submitted. But the backend throws the following error:

Error: {'phone': ['Phone number should only contain digits 0-9', 'Ensure this value has at most 13 characters (it has 15).']}

So I think it'd be better to skip the phone number verification on the backend for now and look into a better alternative for phone number validation.

BrainBuzzer commented 3 years ago

Maybe phonenumbers library can be used for proper validation.

nikochiko commented 3 years ago

ah, alright. i'll add that right away

nikochiko commented 3 years ago

I've added a very rough implementation for that which will just do for the time being. We should add a better converter to a final presentable format in line with the frontend. And we can just store the final format directly in the db

BrainBuzzer commented 3 years ago

I keep getting this error from the backend:

Error: can only concatenate str (not "int") to str
nikochiko commented 3 years ago

Aah. Fixing it. I made some string formatting changes after testing and it messed up right there.

Country code will always be there from frontend input right?

On Mon, Oct 26, 2020, 10:44 Aditya Giri notifications@github.com wrote:

I keep getting this error from the backend:

Error: can only concatenate str (not "int") to str

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hyperlog/hyperlog-backend/pull/114#issuecomment-716308012, or unsubscribe https://github.com/notifications/unsubscribe-auth/AI7MKYOJE55RQC4VFQI54RDSMUAUFANCNFSM4SM22KSA .

BrainBuzzer commented 3 years ago

Yeah, frontend will provide the country code.

BrainBuzzer commented 3 years ago

I get a different error now:

Unhandled Rejection (Error): GraphQL error: value too long for type character varying(13)
BrainBuzzer commented 3 years ago

I guess the default limit for the database is still not changed.

nikochiko commented 3 years ago

Oh, yeah. You'll have to rerun migrations. This should work - in profiles/migrations/005_contactinfo.py, change the max_length in line 22 to something else, and then run python manage.py makemigrations and python manage.py migrate. (after that you can undo the changes in profiles/migrations)

nikochiko commented 3 years ago

I had to delete the previous migration file and rerun it twice because of errors for this limit, and for the validator function. And then django thought it was up-to-date because the new migrations were in the same file