GSoC 2015 - 2214 Dashboard as main page

[wadsashika]

JIRA Ticket - https://issues.apache.org/jira/browse/COUCHDB-2214

Added a widget that shows active tasks to the dashboard. In here I reused functions in Active task module. I did not implement test cases yet.

[robertkowalski]

please remove the space around the = when writing markup

[robertkowalski]

I think we don't need that file

[robertkowalski]

looks really really good! added some notes...

[wadsashika]

@robertkowalski I did the changes you and @michellephung suggested.
And fixed the auto updating the active task list issue that had in the previous commit. Now I started to work on test cases as you mentioned above. I will push those changes as soon as I finished.

[benkeen]

This is interesting. @robertkowalski, I thought the pattern was to use action to update the store and rely entirely on listening to the store changes to get the changes into the component?

[robertkowalski]

yes @benkeen you describe the way to go, this has to be changed

[wadsashika]

@benkeen @robertkowalski
Can you explain little bit more about this? I just follow the implementation of ActiveTaskWidget. So I am not clear about what you are suggesting. If you can, it will be very helpful specially for future works.

[benkeen]

Sure! Sorry for the wait in responding. This'll take a bit of explanation, so sorry for the long message.

One of the ideas in the flux pattern (which we use in Fauxton) is to have one-way communication as shown in the diagram here. Information flows one-way, like so:

1. User clicks on something in a React component,
2. the component fires an action (in an actions.js file),
3. the action dispatches an event (for us, that's the FauxtonAPI.dispatch() call),
4. stores listen to these events (in their dispatch() methods), change the content of the store, then notify anyone that's interested that the store has changed.
5. finally, it comes full circle. React components listen for changes in the store by listening to their change events. That's the purpose of the storeName.on('change', this.onChange) lines.

You're already doing this in your DashboardController component, just not for every item in its state.

So why do all this. The benefit is that it standardizes how data moves around the application, and keeps things simple - regardless of how much bigger the application gets. This is pretty cool.

Here's a simple example: imagine if a user shrunk/expanded the main sidebar, and multiple components in the page needed to know about it to make use of the new space. Maybe one was a graph and needed to redraw for the extra space, and maybe another component could switch from "basic" to "advanced" view or something.

With flux, you can just publish the single event, then each store could listen for it, change whatever data was needed internally, then notify any components that was listening: and they would then have the choice to rerender or not, based on what changed. This is basic "pub/sub": allowing you to keep code loosely coupled, but still communicate. By setting content on the store directly like you're doing here, it ties the component to the individual store and blocks out any other stores which may be interested in the event down the road. Now true, that may be unimportant in certain cases, but by doing it consistently everywhere, it makes the whole application super, super easy to follow (albeit a bit verbose at times!).

So what you need to do is replace the this.state.setPolling(); and this.state.clearPolling(); lines with Actions.setPolling() and Actions.clearPolling() methods (and include your actions.js file in your components.react.jsx file as a dependency). In the actions.js file, add those methods, which just dispatch two new event - just like you're already doing there - defined in your actiontypes.js file. Then update the store's dispatch() method to listen to them, and do a this.triggerChange();. Lastly you can remove setPolling and clearPolling from your component's getStoreState() method.

It feels a bit weird at first adding in all this extra code for something that works already, but it'll be better down the road!

Let me know if I'm not clear on anything. I probably didn't do great justice to the Flux pattern, but that's the gist of it. :)

[wadsashika]

@benkeen
Thank you very much for your detailed explanation. Now I am clear about how flux works and how we should apply it in the project :)

[benkeen]

@wadsashika, this looks terrific! Sorry for all my nit-picky comments above. Luckily I'm in a different country so can't hurl your coffee mug at me.

[wadsashika]

@benkeen I think it would be my phone :D anyway thank you very much for the comments. It really helps to get a better idea of the code. When I read some comments, I wonder that some mistakes you pointed out are already in the code. Because when I implement this, I referred activeTask module. In my first commit, I directly used active task store to get the list of active tasks. But later I took those functions to a separate store in dashboard as a suggestion made by @michellephung . I'll do the changes you pointed out and push those changes.

[robertkowalski]

hmm wait a sec... why are we copy pasting that code over and are not reusing the code?

[michellephung]

@wadsashika you can include the 'Active Tasks Store' in your 'Active Tasks Widgets Store', to get access to the functions.
Then you can put any additional widget functions it's own 'Active Tasks Widgets Store', that uses data that you get from the 'Active Tasks Store'.

Active Tasks Store <-- active tasks functions only
Active Tasks Widgets Store <-- active tasks widget functions only

I'm thinking eventually you will have several 'X Widget Stores', which will include (require) the stores from their respective addons, and you will write your widget-specific functions in those.

For example, you might have a 'most recent databases widget', which would have it's own store file, and that 'most recent database widget store' would have custom functions that you write that are only for the widget.

Another option, would be for you to write your custom widget functions directly into each of the add-ons stores, and then call the functions from your dashboard jsx file.

@benkeen @robertkowalski. is one option better than another here? I thought having the widgets each having their own stores would mean that they'd be easier to debug, and later customize or extend.

[benkeen]

@wadsashika that's a fair point. But I wouldn't worry: I don't think any of the points I make above are actual bugs, just stylistic. And none are particularly egregious, I'm just giving you a hard time because you're new and hey, that's my job here. ;)

Internally, we mix and match who reviews who's code, and everyone has their own viewpoints. That's the awesome part about code reviews: you always get feedback that you won't have thought of, and everyone's different. So you're right, some of these things are already in the code but I wouldn't worry too much about it. We work as a group. And of course: you're always welcome to disagree! If Michelle or Robert were here in Vancouver, I'm pretty sure I'd have demanded settling an argument by an arm wrestle or something.

@michellephung, @robertkowalski that's a fair point Robert. But yeah I'm not averse keeping the widget separate for the moment. Once it's a little more settled we could look at extracting shared code if there are big chunks. The parts I looked at looked like just pieces here and there, which isn't so bad.

[michellephung]

rename all of these

[michellephung]

hi @wadsashika try looking here:
https://github.com/apache/couchdb-fauxton/blob/master/app/addons/databases/resources.js#L128

this is where from the 'All Databases' page, the column is getting the number of docs in each database from,
and correspondingly, https://github.com/apache/couchdb-fauxton/blob/master/app/addons/databases/components.react.jsx#L137 is where it is rendered in React

[michellephung]

Hi,
I opened a PR (wadsashika#1) for grabbing the total docs in the source and target databases.

There are some gaps I left open in the PR, that you should watch out for:

• This will only be good for local DBs, so you will still have make a function in that case that the target/source is a remote database
• I noticed that sometimes you can have more documents written than there are total in the source db, so the percentage will not really be accurate. Any ideas for how we might solve this one? We may have to rethink our strategy on this.
• When I grab the task object in the store, I am only looking for the first object the list that matches the databaseName. But, in case you have a source name like 'def', and there exists 2 databases named 'abcdef', and 'defghi', you might grab data from the wrong source. You'll have to do some error handling here.
• I left the source functions separate from the target functions for clarity, but you should combine them so that they are one function. They do the same thing.

[wadsashika]

Hi @michellephung,
Thank you very much for your great help :) I think I can carry on this with the start you have given. I will go through the noted points that you have mentioned above.

[michellephung]

Hey!

The left/right arrows look great! I love the animation and that scrolling also works.

more things (sorry!) to fix:

• the percentage should always be about 3~5 numbers, sometimes it shows as a long string, when the division gives a long decimal (0.8363201911589008% Complete)
• add a label 'non-continuous' underneath for the replications that are not continuous
• clicking away from the dashboard page, and back will show outdated cards, lets update each time we arrive at that page, also, 5 seconds...seems long to me now. Lets try 1 second instead :D
• some error handling for the remote databases, it's showing up as 'Nan%'
• at smaller widths, the right arrow doesn't scroll over to see the next card
• for remote dbs, we should have two lines to show the remote address, so I suppose for the local DBs can do this as well (show two lines), but keep it from overlapping the sides of the card with padding still. (hint: raise the height and see if the string breaks itself up as needed)
• I also added a comment to line 75 app/addons/dashboard/assets/less/dashboard.less, which should make sure the text is not crowding the card

instead of re-adding app/addons/dashboard/tests/fakeActiveTaskResponseForWidget.js, you can include the fake-data file from the activetasks folder.

Tests look like a good start, but you should write tests for each function you create, and make sure the function does what it's supposed to do. (you can do this by passing it some data, and then asserting the results). The travis tests (the red X next to your commit) wont pass until it reaches the all_dbs pages on default. Lets put on hold the dashboard being the default page on initialization, until we can re-write the nightwatch tests, or think of an alternative.

Good work this week :D

[michellephung]

you can add this line:
!app/addons/dashboard
to your .gitignore file, so your folder shows up when you push :D, also when you merge this in,
the repo's .gitignore file will update with the new folder, and include it in the next changes

[michellephung]

something like this:
https://jsfiddle.net/ozn51bd9/

[asfbot]

Ayola Jayamaha on dev@couchdb.apache.org replies:
Hi,

I would like to contribute to this project as time permits. Hope you would
assist me in giving ideas, guidance and feedback.

Thank you in advance!

[Humbedooh]

Abandoned issue. Closing here.