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

Pool creation succeeds but refresh icon(ajax-loader) still exists #1563

Closed
priyaganti opened this Issue Dec 2, 2016 · 6 comments

Comments

Projects
None yet
4 participants
@priyaganti
Contributor

priyaganti commented Dec 2, 2016

Even though the functionality works, the UX is bad because of the refresh icon.
Steps to reproduce:

  1. The Icon starts rotation as soon as the user enters in the add pool view.
  2. Fill up the fields and click submit. New Pool is created taking to the pools view.
  3. The refresh icon still exists and goes away only when you refresh the page a couple of times.

Adding few snapshots to explain problem in better fashion.

Add pool View
addpool_refreshicon

Pools View
pools_refreshicon

Console Error
addpool_consoleerror

@schakrava schakrava added the bug label Dec 2, 2016

@schakrava schakrava added this to the Pinnacles milestone Dec 2, 2016

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Dec 6, 2016

Member

Had a quick look at this but unfortunately didn't get anyway. Only that it concerns the code added in:
pr #1460 but I don't think it's been happening since then, just the last few testing channel versions; although this is just from memory.

The indicated console error can be triggered by just clicking on the "Create Pool" button and is perfectly reproducible. Had a look as this is potentially holding up my own issues as it would be better for this to be error free prior to me making additional changes in this same area.

Any more pointers from anyone, seems like it's going to be something simple I'm just not seeing?

Member

phillxnet commented Dec 6, 2016

Had a quick look at this but unfortunately didn't get anyway. Only that it concerns the code added in:
pr #1460 but I don't think it's been happening since then, just the last few testing channel versions; although this is just from memory.

The indicated console error can be triggered by just clicking on the "Create Pool" button and is perfectly reproducible. Had a look as this is potentially holding up my own issues as it would be better for this to be error free prior to me making additional changes in this same area.

Any more pointers from anyone, seems like it's going to be something simple I'm just not seeing?

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Dec 6, 2016

Member

Hi @priyaganti @phillxnet & @schakrava ,
my 2 cents:
main error is that 404, because 'match' of null raised by the xhr request failing with 404, that api/pools/usage_bound?raid_level=single is wrong url is wrong. @phillxnet any idea with the right url?
M.

Member

MFlyer commented Dec 6, 2016

Hi @priyaganti @phillxnet & @schakrava ,
my 2 cents:
main error is that 404, because 'match' of null raised by the xhr request failing with 404, that api/pools/usage_bound?raid_level=single is wrong url is wrong. @phillxnet any idea with the right url?
M.

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Dec 7, 2016

Member

Amend my last message, can confirm @phillxnet idea about #1460, while accessing create pool page got this on rockstor.log:
[07/Dec/2016 14:03:55] ERROR [storageadmin.views.pool:470] Exception while updating disk state: 'PoolDetailView' object has no attribute '_update_disk_state'

And finally found why we have this messy one:
aaa1963 and cafda50 flushed mods @sfranzen had over #1460

Member

MFlyer commented Dec 7, 2016

Amend my last message, can confirm @phillxnet idea about #1460, while accessing create pool page got this on rockstor.log:
[07/Dec/2016 14:03:55] ERROR [storageadmin.views.pool:470] Exception while updating disk state: 'PoolDetailView' object has no attribute '_update_disk_state'

And finally found why we have this messy one:
aaa1963 and cafda50 flushed mods @sfranzen had over #1460

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Dec 7, 2016

Member

@MFlyer Well done and thanks for diving into this one, I am still pretty rubbish in this area and just didn't see it.

Member

phillxnet commented Dec 7, 2016

@MFlyer Well done and thanks for diving into this one, I am still pretty rubbish in this area and just didn't see it.

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Dec 7, 2016

Member

and just didn't see it.

Once again @phillxnet , like on another recent issue, I was just lucky: while checking @sfranzen PR and my pools.py url file noticed missing rows ;)

Member

MFlyer commented Dec 7, 2016

and just didn't see it.

Once again @phillxnet , like on another recent issue, I was just lucky: while checking @sfranzen PR and my pools.py url file noticed missing rows ;)

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Dec 7, 2016

Member

@MFlyer I've just confirmed your fix here on my current dev branch by the way. Will update the pr with this.

I'm chuffed this is sorted; at least as far as the symptoms I've noticed are concerned.

@priyaganti It looks like @MFlyer 's patch also sorts the lingering refresh icon as well, at least from my test here.

Member

phillxnet commented Dec 7, 2016

@MFlyer I've just confirmed your fix here on my current dev branch by the way. Will update the pr with this.

I'm chuffed this is sorted; at least as far as the symptoms I've noticed are concerned.

@priyaganti It looks like @MFlyer 's patch also sorts the lingering refresh icon as well, at least from my test here.

@schakrava schakrava closed this in 5f6d7c7 Dec 12, 2016

schakrava added a commit that referenced this issue Dec 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment