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 the queries to be stored as a dict. #994

Merged
merged 1 commit into from Aug 23, 2016
Merged

Refactor the queries to be stored as a dict. #994

merged 1 commit into from Aug 23, 2016

Conversation

bkyryliuk
Copy link
Member

@mistercrunch, @ascott - please take a look.

I refactored the redux store to store the queries as dictionary.
I will make the synchronisation logic with the server a bit easier.

@@ -9,12 +9,17 @@ import { Alert } from 'react-bootstrap';

const QueryHistory = (props) => {
const activeQeId = props.tabHistory[props.tabHistory.length - 1];
const queries = props.queries.filter((q) => (q.sqlEditorId === activeQeId));
if (queries.length > 0) {
var queries_array = []
Copy link
Contributor

Choose a reason for hiding this comment

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

this is probably failing in the linter, should be const queriesArray = [];
this is the style guide we are using.

Copy link
Member

Choose a reason for hiding this comment

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

Also camelCase

@ascott
Copy link
Contributor

ascott commented Aug 23, 2016

i think we were reviewing at the same time @mistercrunch :)

@bkyryliuk
Copy link
Member Author

JS tests are fine, carapal branch is broken - I'll fix it in the separate PR.
I'll merge it, feel free to publish other comments - I will fix them in the #991

screen shot 2016-08-23 at 3 37 31 pm

@bkyryliuk bkyryliuk merged commit 919ac1a into apache:carapal Aug 23, 2016
this.props.queries.forEach((q) => {
if (q.id === qe.latestQueryId) {
latestQuery = q;
for (var key in this.props.queries) {
Copy link
Member

Choose a reason for hiding this comment

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

no need for a loop!
let latestQuery = this.props.queries[qe.latestQueryId]

Copy link
Member Author

Choose a reason for hiding this comment

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

will do that

mistercrunch added a commit that referenced this pull request Aug 30, 2016
* Carapal react mockup

This is really just a mock up written in React to try different
components. It could become scaffolding to build a prototype, or not.

* Merging in Alanna's theme tweaks for SQL lab

* Tweak the display of the alert message in navbar

* Sketching the middleware refresh for Queries

* Adjustments

* Implement timer sync.

* CTAS

* Refactor the queries to be stored as a dict. (#994)

* Download csv endpoint. (#992)

* CSV download engdpoint.

* Use lower case booleans.

* Replcate loop with the object lookup by key.

* First changes for the sync

* Address comments

* Fix query deletions. Update only the queries from the store.

* Sync queries using tmp_id.

* simplify

* Fix the tests in the carapal. (#1023)

* Sync queries using tmp_id.

* Fix the unit tests

* Bux fixes. Pass 2.

* Tweakin' & linting

* Adding alpha label to the SQL LAb navbar entry

* Fixing the python unit tests
@dpgaspar dpgaspar mentioned this pull request May 14, 2019
3 tasks
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 17, 2021
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 24, 2021
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 25, 2021
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 26, 2021
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Feb 19, 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 🚢 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants