-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix: pagination page size in message list #6254
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
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/dvnhem688 |
app/routes/admin/messages/list.js
Outdated
| 'page[number' : params.page || 1 | ||
| filter : filterOptions, | ||
| 'page[size]' : params.per_page || 100, | ||
| 'page[number]' : params.page || 1 |
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.
What does this change?
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.
I was looking into #3693 then I found the wrong syntax page[number
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.
the problem with #3693 is due to wrong passing of querystring. I have some doubts regarding that to discuss in technical meeting.
Codecov Report
@@ Coverage Diff @@
## development #6254 +/- ##
============================================
Coverage 22.57% 22.58%
============================================
Files 523 519 -4
Lines 5715 5704 -11
Branches 111 112 +1
============================================
- Hits 1290 1288 -2
+ Misses 4398 4389 -9
Partials 27 27
Continue to review full report at Codecov.
|
app/routes/admin/messages/list.js
Outdated
| 'page[number]' : params.page || 1 | ||
| }; | ||
| queryString = this.applySortFilters(queryString, params); | ||
| return this.asArray(this.modelFor('admin.messages', queryString)); |
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.
Actually the queryString should be in 'admin.messeges' with page[size] and page[number]. But how to pass it there?
|
the above is fixed, but sorting and searching is giving error. |
|
|
|
Push the changes, or else how will I debug? |
|
See Message Setting model in server and remove sortable/searchable from all fields which are not in model, like recipient |
|
I have removed recipient and now ony sent_at is sortable. And there is no other field that can be make searchable. So I hid the searchbox. |
|
Is the save button necessary? |
I think no, it's not working right now or it shouldn't be there. |
|
It should not be removed if it was saving the settings, please restore but put it in a proper place |
but it's not saving settings actually. they are kind of fixed. On server side the setting is implemented as |
|
so on refreshing page the settings are set to previous(the above values), no matter they are saved whatever |
|
But notification and mail status are editable |
|
reverted |
|
Move save to right and add padding top |


related #3693
Checklist
developmentbranch.