Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: Set pagination field in request body for server side paginated requests #17405

Merged
merged 4 commits into from
Oct 10, 2022
Merged

fix: Set pagination field in request body for server side paginated requests #17405

merged 4 commits into from
Oct 10, 2022

Conversation

arunvjn
Copy link
Contributor

@arunvjn arunvjn commented Oct 9, 2022 •
edited

Description

Sends paginationField is request body for server side paginated requests.

Fixes #17303

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Manual
  • Cypress (Checks for the existence of paginationField params in request body)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@vercel
Copy link

vercel bot commented Oct 9, 2022 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
appsmith ✅ Ready (Inspect) Visit Preview Oct 9, 2022 at 6:46AM (UTC)

@github-actions github-actions bot added Bug Something isn't working FE Coders Pod Issues related to users writing javascript in appsmith JS Promises Needs Triaging Needs attention from maintainers to triage Release Blocker This issue must be resolved before the release labels Oct 9, 2022
@github-actions
Copy link

github-actions bot commented Oct 9, 2022

Unable to find test scripts. Please add necessary tests to the PR.

@arunvjn
Copy link
Contributor Author

arunvjn commented Oct 9, 2022

/ok-to-test sha=2bd6194

@github-actions
Copy link

github-actions bot commented Oct 9, 2022

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3212947505.
Workflow: Appsmith External Integration Test Workflow.
Commit: 2bd6194.
PR: 17405.

@github-actions
Copy link

github-actions bot commented Oct 9, 2022

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/3212947505.
Commit: ``.
Results:

Click to view performance test results

Run 1 (ms) Run 2 (ms) Run 3 (ms) Run 4 (ms) Run 5 (ms) Minimum (ms) Median (ms) Mean (ms) Range (%) SD.Sample (%) SD.Population (%)
SELECT_CATEGORY
scripting 324.24 336.82 344.74 339.49 374.27 324.24 339.49 343.91 14.55 5.40 4.83
painting 4.59 4.42 6.55 4.05 3.14 3.14 4.42 4.55 74.95 27.47 24.62
rendering 103.75 102.17 110.29 108.59 101.91 101.91 103.75 105.34 7.96 3.65 3.28
BIND_TABLE_DATA
scripting 1068.86 1102.96 1010.71 1110.86 1064.21 1010.71 1068.86 1071.52 9.35 3.70 3.31
painting 18.57 12.39 12.8 17.47 19.29 12.39 17.47 16.1 42.86 20.31 18.20
rendering 782.87 787.28 799.18 811.08 801.87 782.87 799.18 796.46 3.54 1.43 1.28
CLICK_ON_TABLE_ROW
scripting 807.36 875.65 899.03 852.97 870.77 807.36 870.77 861.16 10.64 3.98 3.56
painting 8.7 8.08 9.11 10.9 12.95 8.08 9.11 9.95 48.94 19.90 17.79
rendering 294.66 296.4 295.25 298.13 298.77 294.66 296.4 296.64 1.39 0.60 0.54
UPDATE_POST_TITLE
scripting 944.9 863.66 901.86 778.8 858.93 778.8 863.66 869.63 19.10 7.07 6.32
painting 12.69 16.2 11.08 7.93 12.06 7.93 12.06 11.99 68.97 24.85 22.27
rendering 586.8 602.75 594.76 561.31 570.29 561.31 586.8 583.18 7.11 2.94 2.63
OPEN_MODAL
scripting 457.45 431.76 468.41 437.24 465.25 431.76 457.45 452.02 8.11 3.67 3.29
painting 10.68 16.51 9.49 15.1 12.1 9.49 12.1 12.78 54.93 23.16 20.74
rendering 1192.04 1175.65 1185.46 1159.31 1160.95 1159.31 1175.65 1174.68 2.79 1.24 1.11
CLOSE_MODAL
scripting 194.84 186.38 181.97 219.47 190.24 181.97 190.24 194.58 19.27 7.55 6.76
painting 11.73 13.3 5.22 5.16 5.08 5.08 5.22 8.1 101.48 50.25 44.94
rendering 878.66 891.34 876.78 889.09 885.97 876.78 885.97 884.37 1.65 0.72 0.65
SELECT_WIDGET_MENU_OPEN
scripting 276.57 292.04 287.74 292.24 289.13 276.57 289.13 287.54 5.45 2.24 2.00
painting 5.08 6.19 9.14 9.61 6.35 5.08 6.35 7.27 62.31 27.37 24.48
rendering 620.49 618.09 645.27 668.09 641.78 618.09 641.78 638.74 7.83 3.20 2.86
SELECT_WIDGET_SELECT_OPTION
scripting 167.4 150.78 147.27 154.6 156.9 147.27 154.6 155.39 12.95 4.92 4.41
painting 11.05 3.46 10.87 4.23 10.09 3.46 10.09 7.94 95.59 47.48 42.44
rendering 306.88 312.76 298.6 314.69 303.97 298.6 306.88 307.38 5.23 2.13 1.90

@arunvjn arunvjn changed the title fix: Set pagination field in request params for server side paginated requests fix: Set pagination field in request body for server side paginated requests Oct 9, 2022
@arunvjn
Copy link
Contributor Author

arunvjn commented Oct 9, 2022

/ok-to-test sha=7a5464a

@github-actions
Copy link

github-actions bot commented Oct 9, 2022

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3213194220.
Workflow: Appsmith External Integration Test Workflow.
Commit: 7a5464a.
PR: 17405.

@github-actions
Copy link

github-actions bot commented Oct 9, 2022

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/3213194220.
Commit: ``.
Results:

Click to view performance test results

Run 1 (ms) Run 2 (ms) Run 3 (ms) Run 4 (ms) Run 5 (ms) Minimum (ms) Median (ms) Mean (ms) Range (%) SD.Sample (%) SD.Population (%)
SELECT_CATEGORY
scripting 336.76 329.1 352.41 340.63 326.77 326.77 336.76 337.13 7.61 3.03 2.71
painting 3.62 6.37 4.36 5.27 7.04 3.62 5.27 5.33 64.17 26.27 23.45
rendering 106.96 111.73 112.82 107.74 103.09 103.09 107.74 108.47 8.97 3.60 3.23
BIND_TABLE_DATA
scripting 1099.06 1047.67 993.73 1037.66 1097.29 993.73 1047.67 1055.08 9.98 4.20 3.75
painting 14.02 15.14 17.94 18.72 23.72 14.02 17.94 17.91 54.16 21.11 18.87
rendering 835.29 782.19 794.72 830.27 838.27 782.19 830.27 816.15 6.87 3.16 2.83
CLICK_ON_TABLE_ROW
scripting 893.5 877.62 889.62 936.07 815.96 815.96 889.62 882.55 13.61 4.90 4.39
painting 26.51 12.15 9.03 10.33 19.33 9.03 12.15 15.47 112.99 47.45 42.47
rendering 296.28 308.79 308.47 302.74 303.15 296.28 303.15 303.89 4.12 1.68 1.51
UPDATE_POST_TITLE
scripting 844.15 810.25 823.55 842.13 956.43 810.25 842.13 855.3 17.09 6.81 6.09
painting 8.24 8.3 7.91 10.01 8.22 7.91 8.24 8.54 24.59 9.84 8.78
rendering 598.48 567.68 578.46 590.96 583.23 567.68 583.23 583.76 5.28 2.02 1.81
OPEN_MODAL
scripting 473.73 438.63 457.56 469.9 436.08 436.08 457.56 455.18 8.27 3.81 3.41
painting 12.28 20.95 20.66 14.92 12.3 12.28 14.92 16.22 53.45 26.63 23.80
rendering 1220.14 1167.91 1169.37 1225.69 1179.17 1167.91 1179.17 1192.46 4.85 2.37 2.12
CLOSE_MODAL
scripting 211.73 222.49 180.57 184.8 192.07 180.57 192.07 198.33 21.14 9.09 8.13
painting 4.92 8.3 8.33 12.67 15.91 4.92 8.33 10.03 109.57 42.77 38.29
rendering 929.84 884.66 883.37 901.77 881.12 881.12 884.66 896.15 5.44 2.29 2.05
SELECT_WIDGET_MENU_OPEN
scripting 300.92 288.29 272.43 291.85 296.86 272.43 291.85 290.07 9.82 3.78 3.38
painting 6.1 7.06 10.64 9.76 6.39 6.1 7.06 7.99 56.82 25.91 23.15
rendering 639.73 653.63 631.87 649.45 648.06 631.87 648.06 644.55 3.38 1.35 1.21
SELECT_WIDGET_SELECT_OPTION
scripting 151.81 151.66 157.96 157.89 159.11 151.66 157.89 155.69 4.79 2.34 2.09
painting 3.49 3.7 4.33 5.83 5.1 3.49 4.33 4.49 52.12 21.83 19.38
rendering 306.17 307.49 308.32 318.69 312.53 306.17 308.32 310.64 4.03 1.64 1.46

rishabhrathod01
rishabhrathod01 approved these changes Oct 10, 2022
View reviewed changes
Copy link
Contributor

@rishabhrathod01 rishabhrathod01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Aishwarya-U-R
Aishwarya-U-R reviewed Oct 10, 2022
View reviewed changes
Copy link
Contributor

@Aishwarya-U-R Aishwarya-U-R left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cypress changes looks good, but let's please add validation for previous page also. Goahead now since its a release blocker.

@arunvjn arunvjn added the Test Plan Approved Manual/Cypress tests covers changes made on the PR. Else, add skip-testPlan label if not applicable label Oct 10, 2022
@arunvjn arunvjn merged commit 3c58742 into release Oct 10, 2022
@arunvjn arunvjn deleted the fix/17303-set-pagination-field branch October 10, 2022 06:39
@github-actions github-actions bot added the QA Needs QA attention label Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aishwarya-U-R Aishwarya-U-R Aishwarya-U-R left review comments

@rishabhrathod01 rishabhrathod01 rishabhrathod01 approved these changes

@kamakshibhat-appsmith kamakshibhat-appsmith Awaiting requested review from kamakshibhat-appsmith

@ramsaptami ramsaptami Awaiting requested review from ramsaptami

Assignees
No one assigned
Labels
Bug Something isn't working FE Coders Pod Issues related to users writing javascript in appsmith Needs Triaging Needs attention from maintainers to triage QA Needs QA attention Release Blocker This issue must be resolved before the release Test Plan Approved Manual/Cypress tests covers changes made on the PR. Else, add skip-testPlan label if not applicable
Projects
None yet
Milestone
No milestone
3 participants
@arunvjn @rishabhrathod01 @Aishwarya-U-R