Issue#1195 Improve Pool Delete UX #1436

gkadillak · 2016-09-21T05:44:24Z

Deletion of Pool in Modal

Users are now able to delete a pool from the pool detail page via a force delete checkbox. The size of the share is displayed (rounded to the nearest integer) along with a warning saying that all associated information will be removed.

Also included is a fix where the delete button doesn't show up for the root pool in the pool detail page.

MFlyer

Hi @gkadillak,
can you indent on frontend with 4 spaces instead of tab? I do this and @phillxnet too :)

gkadillak · 2016-09-21T17:44:42Z

Not a problem, @MFlyer! Would that be worth adding to the community contribution docs?

MFlyer · 2016-09-21T18:42:45Z

Would that be worth adding to the community contribution docs?

That would be a good thing!

gkadillak · 2016-09-21T18:49:58Z

Ok I'll make a pull request to the docs as well with that change :)

gkadillak · 2016-09-22T06:50:34Z

@MFlyer take a look and tell me what you think!

sfranzen · 2016-09-22T14:39:59Z

@MFlyer So our "official" JS/HTML coding style is going to be 4 spaces per level? I've been using 2 but don't mind changing it in future PRs.

MFlyer · 2016-09-22T14:52:17Z

Hi @sfranzen,
I assumed a 4 spaces tab looking to other Rockstor files and @phillxnet coding too (4 spaces on python, 4 spaces on js, same indentation).
Do we agree on this?
I think @schakrava should have the last say.

sfranzen · 2016-09-22T15:01:11Z

@MFlyer,
Yes, you're right, except for the couple of dashboard widget views apparently. Since I started by copy/pasting some code together from there I was looking at my files and just seeing 2 spaces. Going with 4 now! 👍

sfranzen · 2016-09-22T18:15:39Z

src/rockstor/storageadmin/static/storageadmin/js/templates/pool/pool_details_layout.jst

+      </div>
+      <div class="modal-body">
+        <div class="messages"></div>
+        <h4>Pool {{pool.name}} will be deleted along with the following shares (and all information associated with shares including Snapshots, NFSExports, Samba configs, SFTP, Replics). Are you sure?</h4>


Spelling: replics -> replicas

schakrava · 2016-09-29T00:05:16Z

Yes, 4 spaces guys! For Python we should be following https://www.python.org/dev/peps/pep-0008/. For JS, I am not sure. Any suggestions @gkadillak @MFlyer @priyaganti ?

gkadillak · 2016-09-29T00:12:20Z

I've seen 2 spaces work well with js but I'm impartial. I think it would be good to find a js styleguide and enforce it.

MFlyer · 2016-09-29T07:12:21Z

My personal opinion:
I'm impartial like @gkadillak , 1+ on 4 spaces indent because of better reading.

@phillxnet @priyaganti ?

priyaganti · 2016-09-30T02:14:13Z

I am not sure about the spaces to be used but I have tested your changes. The force delete feature looks good and works as per the user request. Thanks @gkadillak for taking the initiative.

gkadillak · 2016-09-30T03:25:52Z

Happy to hear that @priyaganti ! Thanks for giving it a shot :)

schakrava · 2016-09-30T04:11:29Z

Hey @gkadillak. Nicely done. I just tested your changes and it all looks good. I just have 2 comments.

The behavior of delete button in the pools table view is still the old one. Please update it. See if you can reuse the same template/modal for both views.
Update the indentation to 4 spaces while you are at it :)

gkadillak · 2016-09-30T04:17:58Z

4 spaces it is! I'll update the table view behavior as well.

gkadillak · 2016-10-31T02:19:32Z

@schakrava finally finished up this work. Can you take a look? Thanks!

schakrava · 2016-11-10T13:55:49Z

Hey @gkadillak, sorry for the delay but just got to test this morning. You'll notice that I pushed to your fork, mostly rebase related stuff to save you time.

It's still not working for me. Both in the details view and table view confirmation modals, list of shares is not shown even when I have one in the Pool. Can you take a second look at that logic?

gkadillak · 2016-11-10T18:43:17Z

Yep I'll take another look!

* Fixed /api/pool/<pname> form throwing error stating that 'pool_name' is not a valid attribute for a Pool object

* Reformatted the entire rockstor.js file after noticing the spacing wasn't uniform. To the best of my knowledge, this is in line with the spacing of the rest of the codebase.

* Fetch the given shares for a pool to display pertinent information.

* When deleting a pool, show all of the shares associated with the pool, along with their size in GB

For emacs users, in order to remove tabs from a file, select the entire file (C-x h) and then (M-x untabify).

* Reformatted the entire rockstor.js file after noticing the spacing wasn't uniform. To the best of my knowledge, this is in line with the spacing of the rest of the codebase.

* Fetch the given shares for a pool to display pertinent information.

* When deleting a pool, show all of the shares associated with the pool, along with their size in GB

For emacs users, in order to remove tabs from a file, select the entire file (C-x h) and then (M-x untabify).

Show the same delete modal as the pool description page. Also reload the page after the deletion.

schakrava · 2016-11-16T13:31:07Z

@gkadillak Are you able to reproduce the bug after your rebase?

gkadillak · 2016-11-16T19:01:34Z

Unfortunately, no. Is there anything in the console?

schakrava · 2016-11-19T01:39:43Z

Thanks for your work @gkadillak . I had to make a few changes to get it all working, but we are good now.

gkadillak changed the title ~~1195 improve pool delete ux~~ Issue#1195 Improve Pool Delete UX Sep 21, 2016

MFlyer suggested changes Sep 21, 2016

View reviewed changes

sfranzen suggested changes Sep 22, 2016

View reviewed changes

gkadillak added 10 commits November 15, 2016 20:32

Fix /api/pool/<pname> from throwing error

89d018b

* Fixed /api/pool/<pname> form throwing error stating that 'pool_name' is not a valid attribute for a Pool object

Reformat base js view

71c6885

* Reformatted the entire rockstor.js file after noticing the spacing wasn't uniform. To the best of my knowledge, this is in line with the spacing of the rest of the codebase.

Pass down collection of shares

aaa1963

* Fetch the given shares for a pool to display pertinent information.

Show share size in delete modal

67b7ae3

* When deleting a pool, show all of the shares associated with the pool, along with their size in GB

Tabs are evil

ee1f58a

For emacs users, in order to remove tabs from a file, select the entire file (C-x h) and then (M-x untabify).

Small spacing changes

60abce8

Functions spaced awkwardly

681fe46

Reformat base js view

988e6ca

* Reformatted the entire rockstor.js file after noticing the spacing wasn't uniform. To the best of my knowledge, this is in line with the spacing of the rest of the codebase.

Pass down collection of shares

cafda50

* Fetch the given shares for a pool to display pertinent information.

Show share size in delete modal

11901b3

* When deleting a pool, show all of the shares associated with the pool, along with their size in GB

gkadillak added 5 commits November 15, 2016 20:45

Tabs are evil

664d8e6

For emacs users, in order to remove tabs from a file, select the entire file (C-x h) and then (M-x untabify).

Small spacing changes

62517ac

Functions spaced awkwardly

689e4a8

Update /pools Delete Modal

245c90b

Show the same delete modal as the pool description page. Also reload the page after the deletion.

Update /pool delete modal to be same as /pool/<pool name>

8f7f86c

schakrava merged commit 8f7f86c into rockstor:master Nov 19, 2016

schakrava mentioned this pull request Nov 19, 2016

Improve Pool delete UX #1195

Closed

phillxnet mentioned this pull request Jan 23, 2017

intended internal disk update fails #1623

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue#1195 Improve Pool Delete UX #1436

Issue#1195 Improve Pool Delete UX #1436

gkadillak commented Sep 21, 2016 •

edited

Loading

MFlyer left a comment

gkadillak commented Sep 21, 2016

MFlyer commented Sep 21, 2016

gkadillak commented Sep 21, 2016

gkadillak commented Sep 22, 2016

sfranzen commented Sep 22, 2016

MFlyer commented Sep 22, 2016

sfranzen commented Sep 22, 2016

sfranzen Sep 22, 2016

schakrava commented Sep 29, 2016 •

edited

Loading

gkadillak commented Sep 29, 2016 •

edited

Loading

MFlyer commented Sep 29, 2016

priyaganti commented Sep 30, 2016

gkadillak commented Sep 30, 2016

schakrava commented Sep 30, 2016

gkadillak commented Sep 30, 2016

gkadillak commented Oct 31, 2016

schakrava commented Nov 10, 2016 •

edited

Loading

gkadillak commented Nov 10, 2016

schakrava commented Nov 16, 2016 •

edited

Loading

gkadillak commented Nov 16, 2016

schakrava commented Nov 19, 2016

Issue#1195 Improve Pool Delete UX #1436

Issue#1195 Improve Pool Delete UX #1436

Conversation

gkadillak commented Sep 21, 2016 • edited Loading

Deletion of Pool in Modal

MFlyer left a comment

Choose a reason for hiding this comment

gkadillak commented Sep 21, 2016

MFlyer commented Sep 21, 2016

gkadillak commented Sep 21, 2016

gkadillak commented Sep 22, 2016

sfranzen commented Sep 22, 2016

MFlyer commented Sep 22, 2016

sfranzen commented Sep 22, 2016

sfranzen Sep 22, 2016

Choose a reason for hiding this comment

schakrava commented Sep 29, 2016 • edited Loading

gkadillak commented Sep 29, 2016 • edited Loading

MFlyer commented Sep 29, 2016

priyaganti commented Sep 30, 2016

gkadillak commented Sep 30, 2016

schakrava commented Sep 30, 2016

gkadillak commented Sep 30, 2016

gkadillak commented Oct 31, 2016

schakrava commented Nov 10, 2016 • edited Loading

gkadillak commented Nov 10, 2016

schakrava commented Nov 16, 2016 • edited Loading

gkadillak commented Nov 16, 2016

schakrava commented Nov 19, 2016

gkadillak commented Sep 21, 2016 •

edited

Loading

schakrava commented Sep 29, 2016 •

edited

Loading

gkadillak commented Sep 29, 2016 •

edited

Loading

schakrava commented Nov 10, 2016 •

edited

Loading

schakrava commented Nov 16, 2016 •

edited

Loading