search

emoncms / group

In development: Emoncms groups module
GNU Affero General Public License v3.0
4 stars 8 forks source link

Improve error & success handling #49

Closed takkaria closed 6 years ago

takkaria commented 6 years ago

As part of my capabilities work I wanted to make it so that you would get an error message if you didn't have permission to do something. Instead of repeating code all over the place, it seemed to make sense to put this in the group.js file.

At the same time it seemed to make sense to shift to using asynchronous HTTP requests since the JavaScript console warns about synchronous requests being deprecated. So I did some refactoring work to centralise group API requests.

And then the second commit shows success messages when API calls are successful, to give the user extra feedback on what has happened.

I have done some testing with this but I would not be surprised if it introduces bugs!

cagabi commented 6 years ago

Anna, this all looks very good. Great job :) What I don't know is how to proceed. Tomorrow is my last day before going on holidays and there are so many changes that as you say probably some bugs have been introduced so I would like to be around when we do the merge. I am working on setting up the test suite, almost ready, which is exactly what we need for something like this. But there will not be tests ready until I come back. So saying all of this I would like to hold on for the merge. What do you think?

takkaria commented 6 years ago

Yes, I'm happy to wait - it is a big change to make without proper testing!