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

refactor saveDashboardAndCards thunk to use new PUT endpoint for dashcards #29762

Merged
merged 1 commit into from
Apr 6, 2023

Conversation

EmmadUsmani
Copy link
Contributor

@EmmadUsmani EmmadUsmani commented Apr 3, 2023

Merging into BE PR branch dashboard-bulk-create-delete.

Part of epic #29502

Description

Before updating our API requests to introduce the notion of tabs, we decided to first refactor the existing dashboard edit flow. Instead of using separate POST and DELETE requests for the individual cards being added and removed, we updated to PUT endpoint to handle adding and removing all cards at once.

How to verify

Open a dashboard -> edit -> add and remove cards -> save

Demo

Screen.Recording.2023-04-03.at.9.44.10.AM.mov

Checklist

  • Tests have been added/updated to cover changes in this PR

This change is Reviewable

@github-advanced-security
Copy link

You have successfully added a new CodeQL configuration .github/workflows/codeql.yml:analyze. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@EmmadUsmani EmmadUsmani changed the base branch from tabs-2 to master April 3, 2023 16:57
@EmmadUsmani EmmadUsmani changed the title (2.5/n) refactor saveDashboardAndCards thunk to use new PUT endpoint for dashcards [WIP, don't review] (2.5/n) refactor saveDashboardAndCards thunk to use new PUT endpoint for dashcards Apr 3, 2023
@EmmadUsmani EmmadUsmani changed the base branch from master to dashboard-bulk-create-delete April 6, 2023 18:29
@EmmadUsmani EmmadUsmani changed the title [WIP, don't review] (2.5/n) refactor saveDashboardAndCards thunk to use new PUT endpoint for dashcards refactor saveDashboardAndCards thunk to use new PUT endpoint for dashcards Apr 6, 2023
@EmmadUsmani EmmadUsmani self-assigned this Apr 6, 2023
@EmmadUsmani EmmadUsmani marked this pull request as ready for review April 6, 2023 19:01
@EmmadUsmani EmmadUsmani merged commit af798f3 into dashboard-bulk-create-delete Apr 6, 2023
71 of 72 checks passed
@EmmadUsmani EmmadUsmani deleted the tabs-2.5-put-cards branch April 6, 2023 19:01
@deploysentinel
Copy link

deploysentinel bot commented Apr 6, 2023

Current Cypress Test Results Summary

✅ 1582 Passing - ❌ 3 Failing - ⚠️ 9 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 04/06/2023 07:52:52pm UTC)

Run Details

Running Workflow E2E Tests on Github Actions

Commit: 4a04e2c59fd5f3107a5b909879b49753601e2f46

Started: 04/06/2023 07:21:02pm UTC

❌ Failures

📄   e2e/test/scenarios/visualizations/line_chart.cy.spec.js • 2 Failures

Top 1 Common Error Messages

Timed out retrying after 4000ms: Expected to find element: `.popover[data-state~='visible']`, but never found it.

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
scenarios > visualizations > line chart tooltip of combined dashboard cards (multi-series) should show the correct column title (#16249 custom expression names (#16249-1)
Retry 5Retry 4Retry 3Retry 2Retry 1Initial Attempt
Error: Timed out retrying after 4000ms: Expected to find element: `.popover[data-state~...
Timed out retrying after 4000ms: Expected to find element: `.popover[data-state~='visible']`, but never found it.
4.46% (14) 14 / 314 runs
failed over last 7 days
0% (0) 0 / 314 runs
flaked over last 7 days
scenarios > visualizations > line chart tooltip of combined dashboard cards (multi-series) should show the correct column title (#16249 regular column names (#16249-2)
Retry 5Retry 4Retry 3Retry 2Retry 1Initial Attempt
Error: Timed out retrying after 4000ms: Expected to find element: `.popover[data-state~...
Timed out retrying after 4000ms: Expected to find element: `.popover[data-state~='visible']`, but never found it.
4.46% (14) 14 / 314 runs
failed over last 7 days
0% (0) 0 / 314 runs
flaked over last 7 days

📄   e2e/test/scenarios/downloads/downloads.cy.spec.js • 1 Failure

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
scenarios > question > download png images from dashboards
Retry 5Retry 4Retry 3Retry 2Retry 1Initial Attempt
Error: `cy.request()` failed on:...
`cy.request()` failed on:

http://localhost:4000/api/dashboard/2/cards

The response we received from your web server was:

  > 404: Not Found

This was considered a failure because the status code was not `2xx` or `3xx`.

If you do not want status codes to cause failures pass the option: `failOnStatusCode: false`

-----------------------------------------------------------

The request we sent was:

Method: POST
URL: http://localhost:4000/api/dashboard/2/cards
Headers: {
  "Connection": "keep-alive",
  "user-agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/111.0.5563.146 Safari/537.36",
  "accept": "*/*",
  "cookie": "metabase.TIMEOUT=alive; metabase.DEVICE=04047af9-a4a6-47cd-95a8-d652d1232f0e; metabase.SESSION=471f388c-4cc9-44eb-a099-bbc6f453eb88",
  "accept-encoding": "gzip, deflate",
  "content-type": "application/json",
  "content-length": 50
}
Body: {"cardId":4,"row":0,"col":0,"size_x":8,"size_y":6}

-----------------------------------------------------------

The response we got was:

Status: 404 - Not Found
Headers: {
  "x-frame-options": "DENY",
  "x-xss-protection": "1; mode=block",
  "last-modified": "Thu, 6 Apr 2023 12:22:45 -0700",
  "strict-transport-security": "max-age=31536000",
  "set-cookie": [
    "metabase.TIMEOUT=alive;SameSite=Lax;Path=/;Max-Age=1209600"
  ],
  "x-permitted-cross-domain-policies": "none",
  "cache-control": "max-age=0, no-cache, must-revalidate, proxy-revalidate",
  "x-content-type-options": "nosniff",
  "content-security-policy": "default-src 'none'; script-src 'self' 'unsafe-eval' https://maps.google.com https://accounts.google.com    'sha256-K2AkR/jTLsGV8PyzWha7/ey1iaD9c5jWRYwa++ZlMZc=' 'sha256-ib2/2v5zC6gGM6Ety7iYgBUvpy/caRX9xV/pzzV7hf0=' 'sha256-isH538cVBUY8IMlGYGbWtBwr+cGqkc4mN6nLcA7lUjE='; child-src 'self' https://accounts.google.com; style-src 'self' 'unsafe-inline' https://accounts.google.com; font-src *; img-src * 'self' data:; connect-src 'self' https://accounts.google.com metabase.us10.list-manage.com   ; manifest-src 'self';  frame-ancestors 'none';",
  "content-type": "application/json;charset=utf-8",
  "expires": "Tue, 03 Jul 2001 06:00:00 GMT",
  "content-length": "30",
  "server": "Jetty(11.0.14)"
}
Body: API endpoint does not exist.


https://on.cypress.io/request
4.44% (14) 14 / 315 runs
failed over last 7 days
0% (0) 0 / 315 runs
flaked over last 7 days

⚠️ Flakes

📄   e2e/test/scenarios/collections/revision-history.cy.spec.js • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
revision history curate access admin user should be able to revert a dashboard (#15237)
Retry 2Retry 1Initial Attempt
0.33% (1) 1 / 301 run
failed over last 7 days
29.57% (89) 89 / 301 runs
flaked over last 7 days
revision history curate access normal user should be able to revert a dashboard (#15237)
Retry 1Initial Attempt
0% (0) 0 / 301 runs
failed over last 7 days
35.22% (106) 106 / 301 runs
flaked over last 7 days

📄   e2e/test/scenarios/binning/sql.cy.spec.js • 3 Flakes

Top 1 Common Error Messages

null

3 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
scenarios > binning > from a saved sql question via simple question should work for number
Retry 1Initial Attempt
1.01% (3) 3 / 297 runs
failed over last 7 days
18.52% (55) 55 / 297 runs
flaked over last 7 days
scenarios > binning > from a saved sql question via custom question should work for time series
Retry 1Initial Attempt
1.67% (5) 5 / 299 runs
failed over last 7 days
21.07% (63) 63 / 299 runs
flaked over last 7 days
scenarios > binning > from a saved sql question via custom question should work for number
Retry 1Initial Attempt
1.01% (3) 3 / 297 runs
failed over last 7 days
17.85% (53) 53 / 297 runs
flaked over last 7 days

📄   e2e/test/scenarios/onboarding/setup/setup.cy.spec.js • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
scenarios > setup should send snowplow events
Retry 1Initial Attempt
1.25% (4) 4 / 320 runs
failed over last 7 days
0.31% (1) 1 / 320 run
flaked over last 7 days

📄   e2e/test/scenarios/organization/moderation-question.cy.spec.js • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
scenarios > saved question moderation as an admin should be able to verify and unverify a saved question
Retry 1Initial Attempt
3.82% (12) 12 / 314 runs
failed over last 7 days
85.67% (269) 269 / 314 runs
flaked over last 7 days

📄   e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Write Actions on Dashboards (mysql) Actions Data Types can update various data types via implicit actions
Retry 1Initial Attempt
4.11% (13) 13 / 316 runs
failed over last 7 days
7.28% (23) 23 / 316 runs
flaked over last 7 days

📄   e2e/test/scenarios/models/model-actions.cy.spec.js • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
scenarios > models > actions should allow CRUD operations on model actions
Retry 3Retry 2Retry 1Initial Attempt
5.66% (18) 18 / 318 runs
failed over last 7 days
28.62% (91) 91 / 318 runs
flaked over last 7 days

View Detailed Build Results


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant