[Feature] Added account deletion and suspension

[jjkirkpatrick]

I have added the ability to soft delete and account so you don't need to delete valuable user information, also added account suspension based on timestamps

[panique]

Seems like this reduces the code quality quite much:
https://scrutinizer-ci.com/g/panique/huge/inspections/3b092ee7-e714-422a-a02f-fc13f7347a1a

Before it was:
https://scrutinizer-ci.com/g/panique/huge/

[jjkirkpatrick]

There is currntly no way for the user to suspend or delete and account as that would be an admin feature and we don't have that, and yeah scrutinizer isn't happy with me concatenating the time left on the suspension with the text from Text::get in Session::add, i guess i could split it up but i don't actually see anything wrong with the way i have done this

[panique]

:) Yeah scrutinizer is really bitchy sometimes with totally good code!

Does this feature make sense when there's currently no way to really use it ? What do the others say, any opinions ? Personally I think account stopping / deletion is useful.

[jjkirkpatrick]

i think it's definitely useful, even without an nice flashy admin page peoples account can be suspended and or soft deleted from the database now, which wasn't a possibility before, this just gives that little bit more control over users out of the box. Would love to know what others think

[jjkirkpatrick]

Separated the time left from the Session::Add, Let's see if it's happy with that.

Still say's it got worse, I don't know what it's bitching for any ideas?

I think it's pure basing this on

Complexity                         Complexity                         

Conditions  10                  Conditions  12
Paths   288                        Paths    1152

from the addition of two If statements, this complexity system seems stupid to me.

[panique]

Hmm, I'm unsure as this is basically "dead code", as it's not used anywhere. You know, there's no possibility to delete users or to suspend them in any way yet, and this might confuse people more than it helps them.

I think we should only release fully working features. Other opinions ?

[jjkirkpatrick]

well the features can be accessed by changing data in the database, yes this is not desirable, i can work on an admin feature if you want, however without proper roles it wont be great and it's not really a login feature, however suspensions and deletions of account is, it's a tricky one.

[jjkirkpatrick]

What does this need now to be merged?

[panique]

There's absolutly no possibility to delete a user, and there's also no way to suspend a user right now. Practically this is unused / dead code.

[mtaylorsherwood]

Would it not make more sense to create a basic 'admin' section? Use the existing functions and alter them to allow people to create an account, change username/email. Could tie into the discussion about roles and locking it down away from 'standard' users.

Once that's in place you can look at adding suspension and deletion in a nice clean way without needing to manipulate the database directly.

[jjkirkpatrick]

Added very basic admin feature

[panique]

Thanks, this looks awesome! 👍 I'll have a deeper look in the next weeks and merge this within March 2015.

[ldmusic]

Hi, if I see this correctly, you only restrict the visibility of the link to the new admin page for user with account type "2". But I think you should restrict it in the controler. But I may have missed something.

[jjkirkpatrick]

I would tend to agree, however as there are talks of a roles system that would need to be reworked anyway, up to @panique

[ldmusic]

Hi, you're right. I only wanted to note that, 'cause sometimes user use Developer Versions. Perhaps in the meantime something like

If(Session::get('user_account_type') != 2) Redirect::home();

in AdminController.php can be a workaround.

[jjkirkpatrick]

Added that little bit of restriction to the controller

[ldmusic]

Cool! Thank you!

[panique]

@oisian1 Thank you very much, looks good! This needs some tiny fixes but it's really a great feature!

Interesting security thing, maybe useful for everybody reading this. Doing a redirect will STILL run the code after the redirect (!!), so

if (Session::get('user_account_type') != 2) {
    Redirect::home();
}
echo 123;

will ask the browser to redirect to home AND it will also echo 123 ! Redirects are just a command sent to the browser afaik, and not really something that will stop your code from going on. If the user parses your page without a browser (maybe with curl) you'll definitly see this, try it out :)
Really a weird thing, I was shocked the first time.

[jjkirkpatrick]

If you wan't to put comments through the bits that need fixing in the commits section ill get them sorted

[jjkirkpatrick]

What does this need to be merged?

[panique]

Thank you very much, this awesome feature is now part of the develop branch! I've added some tiny improvements (spelling, code convention, file name change etc). Please gimme some days to test this properly :)