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: New schedule query button #13797

Merged
merged 9 commits into from
Mar 26, 2021

Conversation

AAfghahi
Copy link
Member

SUMMARY

this is a rebase of the first one.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@AAfghahi AAfghahi force-pushed the newScheduleQueryButton branch 2 times, most recently from 06c4144 to ce272ce Compare March 25, 2021 13:51
@codecov
Copy link

codecov bot commented Mar 25, 2021

Codecov Report

Merging #13797 (752e69d) into master (1d5c58d) will increase coverage by 0.69%.
The diff coverage is 28.12%.

❗ Current head 752e69d differs from pull request most recent head a8c1289. Consider uploading reports for the commit a8c1289 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13797      +/-   ##
==========================================
+ Coverage   76.79%   77.49%   +0.69%     
==========================================
  Files         934      934              
  Lines       47201    47206       +5     
  Branches     5876     5882       +6     
==========================================
+ Hits        36248    36581     +333     
+ Misses      10794    10483     -311     
+ Partials      159      142      -17     
Flag Coverage Δ
cypress 56.25% <14.81%> (-1.47%) ⬇️
javascript 63.59% <28.12%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...perset-frontend/src/components/FormLabel/index.tsx 100.00% <ø> (ø)
...tend/src/SqlLab/components/ScheduleQueryButton.tsx 28.12% <28.12%> (ø)
superset-frontend/src/filters/utils.ts 95.23% <0.00%> (-4.77%) ⬇️
...erset-frontend/src/SqlLab/components/ResultSet.tsx 68.64% <0.00%> (-0.30%) ⬇️
superset-frontend/src/explore/exploreUtils.js 69.81% <0.00%> (+0.62%) ⬆️
...perset-frontend/src/views/CRUD/chart/ChartList.tsx 84.49% <0.00%> (+0.77%) ⬆️
...rontend/src/SqlLab/components/AceEditorWrapper.tsx 61.25% <0.00%> (+1.25%) ⬆️
...erset-frontend/src/SqlLab/components/SqlEditor.jsx 56.01% <0.00%> (+1.38%) ⬆️
...set-frontend/src/dashboard/util/getDropPosition.js 90.90% <0.00%> (+1.51%) ⬆️
... and 44 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d5c58d...a8c1289. Read the comment docs.

@hughhhh hughhhh self-requested a review March 26, 2021 13:56
@hughhhh hughhhh merged commit 64f967c into apache:master Mar 26, 2021
@hughhhh hughhhh deleted the newScheduleQueryButton branch March 26, 2021 13:57
import Form, { FormProps, FormValidation } from 'react-jsonschema-form';
import { Col, FormControl, FormGroup, Row } from 'react-bootstrap';
import { t, styled } from '@superset-ui/core';
import * as chrono from 'chrono-node';
Copy link
Member

Choose a reason for hiding this comment

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

@AAfghahi This library is missing type definition and local webpack is throwing TS errors. I'm surprised CI didn't catch this.

Copy link
Member Author

@AAfghahi AAfghahi Apr 1, 2021

Choose a reason for hiding this comment

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

Hey! I was running into the same issue, did you do npm install? That solved it for me. The earlier versions don't have type definitions and the later ones do. One of the changes was to package.json.

Copy link
Member

@ktmud ktmud Apr 5, 2021

Choose a reason for hiding this comment

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

I'm pretty sure I did but I was still seeing the error. Nevertheless, I bumped it again to the latest in package.json and it works.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for bumping the version.


const getJSONSchema = () => {
const jsonSchema = window.featureFlags.SCHEDULED_QUERIES?.JSONSCHEMA;
// parse date-time into usable value (eg, 'today' => `new Date()`)
Copy link
Member

@ktmud ktmud Apr 1, 2021

Choose a reason for hiding this comment

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

For this feature, though, @zhaoyongjie implemented an awesome date parser (in Python) for time filters, @AAfghahi @hughhhh do you think it would make sense to consolidate these to use the same API?

allanco91 pushed a commit to allanco91/superset that referenced this pull request May 21, 2021
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 1.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants