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.

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

fix: reverts private widgets feature #10693

Closed
wants to merge 2 commits into from

Conversation

ohansFavour
Copy link
Contributor

@ohansFavour ohansFavour commented Jan 27, 2022

Description

This PR reverts the private widgets feature introduced in this PR

Fixes #10691
Fixes #10668

Type of change

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

How Has This Been Tested?

Manual

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

Test coverage results 🧪

🔴 Total coverage has decreased
// Code coverage diff between base branch:release and head branch: fix/revert-private-widget-feature 
Status File % Stmts % Branch % Funcs % Lines
🔴 total 54.96 (-0.04) 36.38 (-0.18) 34.86 (-0.07) 55.36 (-0.04)
🟢 app/client/src/sagas/PostEvaluationSagas.ts 16.55 (0.11) 0 (0) 0 (0) 20.17 (0.17)
🔴 app/client/src/sagas/WidgetOperationSagas.tsx 51.32 (-0.22) 38.54 (-0.48) 52.08 (0) 52.79 (0)
🟢 app/client/src/selectors/commentsSelectors.ts 85.25 (1.64) 64.71 (2.95) 73.33 (0) 90.59 (2.35)
🔴 app/client/src/utils/DSLMigrations.ts 73.33 (0.03) 63.5 (-6.06) 71.13 (0) 73.1 (0)
🔴 app/client/src/utils/DynamicBindingUtils.ts 83.52 (-0.55) 69.64 (-1.79) 75.86 (0) 82.82 (-0.62)
🔴 app/client/src/utils/WorkerUtil.ts 88.98 (-0.78) 70.59 (-1.96) 100 (0) 92.38 (-0.95)
🔴 app/client/src/widgets/BaseWidget.tsx 73.98 (-4.88) 35.56 (-8.88) 68.75 (-6.25) 72.88 (-5.09)
🔴 app/client/src/widgets/ListWidget/widget/index.tsx 61.95 (-1.39) 25.89 (-1.47) 56.86 (-1.63) 60.79 (-1.75)
🔴 app/client/src/workers/DataTreeEvaluator.ts 76.26 (-0.09) 57.77 (-0.18) 72.53 (-0.59) 76.17 (-0.11)
🔴 app/client/src/workers/evaluationUtils.ts 57.1 (-2.23) 53.29 (-1.62) 61.11 (-2.68) 54.81 (-2.33)

@vercel
Copy link

vercel bot commented Jan 27, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/get-appsmith/appsmith/HYfKdYLc1jKheugQubdwTfJFJLz5
✅ Preview: https://appsmith-git-fix-revert-private-widget-feature-get-appsmith.vercel.app

@ohansFavour
Copy link
Contributor Author

/ok-to-test sha=88077bd

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1756398978.
Workflow: Appsmith External Integration Test Workflow.
Commit: 88077bd.
PR: 10693.

@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/1756398978.
Commit: 88077bd.
Results:

Click to view performance test results

Run 1 Run 2 Run 3 Run 4 Run 5 Mean SD.Sample SD.Population
Edit input
scripting 358.62 390.98 369.26 363.38 385.51 373.55 3.77 3.37
painting 7.56 35.26 4.35 4.19 23.97 15.07 92.44 82.68
rendering 15.03 18.95 20.81 22.8 17.84 19.09 15.45 13.83
Clear input
scripting 912.52 870.07 915.83 891.61 1256.37 969.28 16.67 14.91
painting 12.77 14.03 24.43 24.16 7.19 16.52 45.70 40.92
rendering 262.58 256.86 257.92 260.31 253.5 258.23 1.34 1.20
Edit input again
scripting 399.72 377.6 874.32 363.37 322.81 467.56 49.00 43.83
painting 21.25 17.1 13.33 6.96 5.76 12.88 51.16 45.81
rendering 23.54 30.38 21.33 26.19 22.89 24.87 14.27 12.75

@ohansFavour
Copy link
Contributor Author

/ok-to-test sha=6bbbb6e

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1756644657.
Workflow: Appsmith External Integration Test Workflow.
Commit: 6bbbb6e.
PR: 10693.

@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/1756644657.
Commit: 6bbbb6e.
Results:

Click to view performance test results

Run 1 Run 2 Run 3 Run 4 Run 5 Mean SD.Sample SD.Population
Click on table row
scripting 2699.51 2228.93 2202.68 2313.55 2275.54 2344.04 8.67 7.75
painting 80.01 33.35 56.88 30.29 35.75 47.26 44.63 39.91
rendering 441.76 306.6 341.77 358.84 307.72 351.34 15.74 14.07
Edit input
scripting 335.67 297.69 275.4 307.39 310.68 305.37 7.15 6.40
painting 11.81 4.61 13.14 18.13 7.02 10.94 48.54 43.42
rendering 16.92 14.31 11.63 12.34 15.21 14.08 15.27 13.64
Clear input
scripting 775.7 765.96 823.06 737.85 777.48 776.01 3.96 3.54
painting 17.17 15.55 12.37 15.71 11.16 14.39 17.51 15.64
rendering 242.6 221.29 234.26 223.87 222.25 228.85 4.05 3.62
Edit input again
scripting 349.51 275 306.44 295.9 293.71 304.11 9.14 8.17
painting 4.99 4.64 6.27 4.29 7.04 5.45 21.28 19.08
rendering 20.64 22.05 21.85 17.97 19.04 20.31 8.71 7.83

@ramsaptami
Copy link
Contributor

Tested both issues on DP and both issues have been resolved
#10691
#10668

@ohansFavour ohansFavour marked this pull request as draft January 28, 2022 07:39
@ohansFavour ohansFavour closed this Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
2 participants