Compass 98 react convert collection #593

satyasinha · 2016-11-14T06:20:55Z

Needs testing and reviewing.

Have noticed a bug where building a query on one collection, applying and switching to another collection leads to the schema store to constantly run.

satyasinha · 2016-11-15T04:42:29Z

the document and explain plan is still running twice when both query and namespace changes are taking place. Still working on this.

rueckstiess

This change also breaks the functional tests again (was expected). Let's try to fix them. It's probably just some new css selectors for the tabs that need to be changed to the new elements.

rueckstiess · 2016-11-17T22:43:26Z

src/app/home/collection.js

@@ -1,5 +1,5 @@
 var View = require('ampersand-view');
-var Action = require('hadron-action');
+// var Action = require('hadron-action');


is ./src/app/home/collection.js still used? If not, you can delete the entire file.

rueckstiess · 2016-11-17T22:44:51Z

src/app/home/index.jade

 .page
  .content.with-sidebar
-    div(data-hook='collection-subview')
+    div(data-hook='collection-new-view')


can you rename it back to collection-view or collection-subview and remove the comment? (and also change it accordingly in ./src/app/home/index.js? We don't want the word new in there because after we merge, old is the new new :-)

rueckstiess · 2016-11-17T22:48:32Z

src/internal-packages/collection/lib/components/index.jsx

+
+    NamespaceStore.listen((ns) => {
+      if (ns && toNS(ns).collection) {
+        this.setState({name: toNS(ns).collection, showView: true, activeTab: this.CollectionStore.getActiveTab()});


We should not directly call functions on a store. The store passes all the required information down to the component as props. If something you need is missing, you should add it to the store state, and retrieve it as this.props.activeTab.

rueckstiess · 2016-11-17T22:52:46Z

src/internal-packages/collection/lib/components/index.jsx

+  }
+
+  onTabClicked(idx) {
+    this.setState({activeTab: idx});


Two issues here:

again, never call store functions directly. Everything needs to go via an action. There should be a setActiveTab(idx) action that the store listens to, and then passes down it's state.

subtlety with setState and props: calling setState() will trigger a render(). But the action (see above) will cause a store change and also trigger a render(), so you render twice. The right way to do this is to use the componentWillReceiveProps() hook and change the state internally there. Let's discuss this one in person.

rueckstiess · 2016-11-17T22:55:41Z

src/internal-packages/crud/lib/component/document-list.jsx

   * Fetch the state when the component mounts.
   */
  componentDidMount() {
+    debug('is doc list being mounted');


remove debug.

rueckstiess · 2016-11-17T22:56:44Z

src/internal-packages/crud/lib/store/reset-document-list-store.js

+    if (filter === null) {
+      return;
+    }
+    debug('reset is called....');


remove debug.

rueckstiess · 2016-11-17T22:56:51Z

src/internal-packages/crud/lib/store/reset-document-list-store.js

          const options = { limit: 20, sort: [[ '_id', 1 ]], readPreference: READ };
          app.dataService.find(NamespaceStore.ns, filter, options, (error, documents) => {
            if (!error) {
+              debug('reset is called.... 2');


and this one :)

rueckstiess · 2016-11-17T22:58:32Z

src/internal-packages/explain/lib/components/compass-explain.jsx

      <div className="compass-explain header-margin">
        <div className="flexbox-fix"></div>
+        <this.queryBar />
        {this.CollectionStore.isReadonly() ? this.renderReadonly() : this.renderComponent()}


Hm. I realize now that Durran also calls store functions directly. This goes against the flux pattern. So I guess you were just following his example. I'd like to use the flux pattern as closely as possible, and will also talk to Durran about it so we are on the same page.

rueckstiess · 2016-11-17T22:59:51Z

src/internal-packages/indexes/lib/store/load-indexes-store.js

 const IndexModel = require('mongodb-index-model');
 const NamespaceStore = require('hadron-reflux-store').NamespaceStore;
-const Action = require('../action/index-actions');
+// const Action = require('../action/index-actions');


remove all lines that are commented out and no longer needed.

rueckstiess · 2016-11-17T22:59:58Z

src/internal-packages/indexes/lib/store/load-indexes-store.js

  init: function() {
    this.CollectionStore = app.appRegistry.getStore('App.CollectionStore');
-    this.listenTo(Action.loadIndexes, this.loadIndexes);
+    // this.listenTo(Action.loadIndexes, this.loadIndexes);


this one too.

satyasinha · 2016-11-18T04:48:16Z

This push fixes most of the above, working on realigning functional tests now

satyasinha · 2016-11-19T12:03:26Z

For some bizarre reason I can't select the query bar reset button in the functional tests. Otherwise the tests seem mostly fine. This should now be reviewed by a few.

pzrq · 2016-11-20T23:43:00Z

@KeyboardTsundoku

satyasinha · 2016-11-20T23:47:07Z

@pzrq good catch, not sure how that disappeared since we wouldn't have touched that

durran · 2016-11-21T00:29:15Z

Just a heads up - the create collection button is visible only if the server is writable. This means only a mongos, standalone, or replica set primary connection would have the button visible. Not sure if that was the case with @pzrq 's comment but something to note.

satyasinha · 2016-11-21T00:31:51Z

It's more the light theme of the top navbar, it has top css property of 60 but the dark theme doesn't.
The database view uses light theme but if a dark theme was used it'll fix it. Seems sort of inconsistent.....

rueckstiess · 2016-11-21T00:42:03Z

oh, yes there should be no difference other than color between light and dark theme.

* COMPASS-98 add options to QueryStore and add QueryChangedStore * fix typo * comments

…d x from schema action/store files

...when updating namespace and schema view was active.

...when updating namespace and schema view was active. cherry-picking deb2133

* COMPASS-98 added query bar to schema * COMPASS-98 added collection component * COMPASS-98 added query bar to CRUD and Explain * COMPASS-98 cleaned up collection package * COMPASS-98 fixing index * COMPASS-98 add options to QueryStore and add QueryChangedStore (#590) * COMPASS-98 add options to QueryStore and add QueryChangedStore * fix typo * comments * pass all other query options to QueryChangedStore. (#592) * COMPASS-98 listening to namespace for crud and indexes init * COMPASS-98 crud, schema and explain listens to query changes + removed x from schema action/store files * COMPASS-98 added fix to Explain plan not resetting on collection change * COMPASS-98 tests working and schema sample bug fixed * COMPASS-98 CRUD and explain working * COMPASS-98 switching between collection and db view * COMPASS-98 set name of collection * COMPASS-98 document & indexes loading on mount * COMPASS-98 made no difference on styling :( * COMPASS-98 removed hanging console.log * COMPASS-98 a slightly safer way of preventing double resets for crud * Header repositioning fix * COMPASS-98 fix bug where minicharts don’t rerender... ...when updating namespace and schema view was active. * COMPASS-98 fix bug where minicharts don’t rerender... ...when updating namespace and schema view was active. cherry-picking deb2133 * fix linter issue. * fix linter issue. * COMPASS-98 minor changes to remove comments * COMPASS-98 removed unused files * COMPASS-98 removed useless flexbox-fixed class * using collection 'store' method instead * removed debugs * full namespace as title instead of just collection name * COMPASS-98 adding id to tabs * 5 passing functional tests! * splitting functional test of query refine and apply * supporting more test cases! * COMPASS-98 most functional tests working * added extra margin top to show the create button * skip reset test instead of commenting out.

pzrq · 2016-11-22T05:11:46Z

Backported to 1.5-releases, see also #611, #558, #600

satyasinha added the work in progress label Nov 14, 2016

satyasinha force-pushed the COMPASS-98-React-Convert-Collection branch from 97103ce to 71d4658 Compare November 15, 2016 04:39

satyasinha force-pushed the COMPASS-98-React-Convert-Collection branch from 71d4658 to d6442a6 Compare November 16, 2016 05:54

rueckstiess reviewed Nov 17, 2016

View reviewed changes

satyasinha force-pushed the COMPASS-98-React-Convert-Collection branch from ba1fcb1 to a7af8c9 Compare November 19, 2016 11:56

satyasinha added ready for review and removed work in progress labels Nov 19, 2016

rueckstiess mentioned this pull request Nov 20, 2016

COMPASS-173 : Tab Ordering - Schema, Documents, Indexes, Explain Plan #609

Closed

satyasinha force-pushed the COMPASS-98-React-Convert-Collection branch from b4a990a to 8afb1f5 Compare November 20, 2016 23:32

KeyboardTsundoku and others added 13 commits November 21, 2016 16:31

COMPASS-98 added query bar to schema

5f8181b

COMPASS-98 added collection component

ffb1ae3

COMPASS-98 added query bar to CRUD and Explain

a7a311f

COMPASS-98 cleaned up collection package

d927098

COMPASS-98 fixing index

e9ce0e8

COMPASS-98 add options to QueryStore and add QueryChangedStore (#590)

777df6e

* COMPASS-98 add options to QueryStore and add QueryChangedStore * fix typo * comments

pass all other query options to QueryChangedStore. (#592)

8566953

COMPASS-98 listening to namespace for crud and indexes init

4e4e591

COMPASS-98 crud, schema and explain listens to query changes + remove…

1cc8e94

…d x from schema action/store files

COMPASS-98 added fix to Explain plan not resetting on collection change

fc263a2

COMPASS-98 tests working and schema sample bug fixed

375e14a

COMPASS-98 CRUD and explain working

14c0163

COMPASS-98 switching between collection and db view

afac12e

Thomas Rueckstiess and others added 19 commits November 21, 2016 16:31

COMPASS-98 fix bug where minicharts don’t rerender...

b430c6d

...when updating namespace and schema view was active.

fix linter issue.

ad7d380

Header repositioning fix

281f288

COMPASS-98 a slightly safer way of preventing double resets for crud

6d20efd

COMPASS-98 fix bug where minicharts don’t rerender...

3dfc08b

...when updating namespace and schema view was active. cherry-picking deb2133

fix linter issue.

bb1a984

COMPASS-98 minor changes to remove comments

1c249de

COMPASS-98 removed unused files

30de520

COMPASS-98 removed useless flexbox-fixed class

3131359

using collection 'store' method instead

44a270b

removed debugs

132281a

full namespace as title instead of just collection name

f179b4c

COMPASS-98 adding id to tabs

fc4c016

5 passing functional tests!

aaef99f

splitting functional test of query refine and apply

cc3326b

supporting more test cases!

45fb7bf

COMPASS-98 most functional tests working

90e5e7d

added extra margin top to show the create button

f00a7d6

skip reset test instead of commenting out.

969b76e

rueckstiess force-pushed the COMPASS-98-React-Convert-Collection branch from 66318c9 to 969b76e Compare November 21, 2016 05:36

rueckstiess merged commit d0f5642 into master Nov 21, 2016

rueckstiess deleted the COMPASS-98-React-Convert-Collection branch November 21, 2016 05:42

rueckstiess added backport:needed and removed ready for review labels Nov 21, 2016

pzrq added backport:done and removed backport:needed labels Nov 22, 2016

This was referenced Nov 22, 2016

Compass 98 eliminate query options #592

Merged

COMPASS-98 add options to QueryStore and add QueryChangedStore #590

Merged

Compass 98 react convert collection #593

Compass 98 react convert collection #593

Uh oh!

Conversation

satyasinha commented Nov 14, 2016

Uh oh!

satyasinha commented Nov 15, 2016

Uh oh!

rueckstiess left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

satyasinha commented Nov 18, 2016

Uh oh!

satyasinha commented Nov 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pzrq commented Nov 20, 2016

Uh oh!

satyasinha commented Nov 20, 2016

Uh oh!

durran commented Nov 21, 2016

Uh oh!

satyasinha commented Nov 21, 2016

Uh oh!

rueckstiess commented Nov 21, 2016

Uh oh!

pzrq commented Nov 22, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

satyasinha commented Nov 19, 2016 •

edited

Loading