Skip to content
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

Add ability to remove sold / bio massed characters from user_group #231

Closed

Conversation

Projects
None yet
3 participants
@herpaderpaldent
Copy link
Contributor

commented Nov 8, 2018

  • will create an intel note entry
  • intel notes are now showed in /configuration/users/edit/

This completes the following issue: eveseat/seat#449

image

Add ability to remove sold / bio massed characters from user_group
* will create an intel note entry
* intel notes are now showed in `/configuration/users/edit/`
@herpaderpaldent

This comment has been minimized.

Copy link
Contributor Author

commented Nov 8, 2018

i shall add that this PR is not aimed to over engineer the whole thing. I wasn't able to update the main_character_id of a group if you deactivate the main_character. This would be a super edge case and could be solved afterwards with impersonating.
It's a very functional solution to aim with sold and bio massed characters. Moving them out of a user group to prevent MissingRefreshToken Exceptions (relevant for SeAT-Groups and Discord-Connector)

herpaderpaldent added some commits Nov 11, 2018

@herpaderpaldent

This comment has been minimized.

Copy link
Contributor Author

commented Nov 11, 2018

Hey @warlof

image

i included our yesterdays discussions, fixed an persisting error from 2.x migration (set-email) and added the ability to activate an user again. I did add the ability to lock seat from registration too:

Show resolved Hide resolved src/resources/views/auth/login.blade.php Outdated
Show resolved Hide resolved src/Http/Controllers/Configuration/UserController.php Outdated
Show resolved Hide resolved src/Http/Controllers/Configuration/UserController.php Outdated
Show resolved Hide resolved src/Http/Controllers/Configuration/UserController.php Outdated
Show resolved Hide resolved src/Http/Controllers/Configuration/UserController.php Outdated
Show resolved Hide resolved src/Http/Controllers/Auth/SsoController.php
Show resolved Hide resolved src/Http/Controllers/Configuration/UserController.php Outdated
Show resolved Hide resolved src/Http/Controllers/Auth/LoginController.php Outdated

herpaderpaldent added some commits Nov 29, 2018

Remove settings
Apply fixes from StyleCI

@warlof warlof added this to the 3.0.12 milestone Dec 15, 2018

@warlof

This comment has been minimized.

Copy link
Member

commented Dec 16, 2018

This PR still need a few changes.
As a standard user, I'm not able to deactivate/remove the character. It should be the case, at least with a permission or a global setting.

  • A disabled user is still able to navigate into SeAT if he's or not already logged-in => disabled account don't have any effect
  • When a disabled user is also a group main, the settings is not updated. At least, we should keep a remaining random character from the group. The best would be to ask the user which remaining character should become a main.
  • The note table from users edit card need an escaped column for Note field - otherwise, plain html will be displayed instead being rendered.
  • When the registration is disabled and I'm attempting to sign-in with a not registered user, I'm redirected to the login page without any information (what append ?)
@herpaderpaldent

This comment has been minimized.

Copy link
Contributor Author

commented Dec 17, 2018

So this PR had caused some mix feelings for me. We had a quite heated discussion on slack although i have the feeling that generally the cause of this is a misconception of the introduced feature.

The feature does:

Add ability to remove sold / bio massed characters from user_group

it does not aim to ban a user from using seat. Therefore:

A disabled user is still able to navigate into SeAT if he's or not already logged-in => disabled account don't have any effect

This is correct and aimed behavior. However only a sold character is able to do so, as a biomassed character won't be able to receive a token via SSO. Disabling a user helps f.e. marking user in the user-list as such a special case:

createdRow: function (row, data) {
if (data.active === false){
$(row).addClass('warning')
}

When a disabled user is also a group main, the settings is not updated. At least, we should keep a remaining random character from the group. The best would be to ask the user which remaining character should become a main.

i have mentioned this in the comment on this PR as i was struggeling with it. I stand that selling/biomassing a character is a super edge chase and could be solved by the admin disabeling it easily.
image

When the registration is disabled and I'm attempting to sign-in with a not registered user, I'm redirected to the login page without any information (what append ?)

This is correct, i forgot to add this to the view: 33bf6af
image

an user come to me and say "this toon has been biomassed"
as a super, I go in users list, disabling the user and saying what the owner told me "sold toon"
the user is now "something"
tomorrow, the owner is sign in with this user
the user is no longer "something"

what's the purpose ?

As mentioned the purpose of this PR is:

Add ability to remove sold / bio massed characters from user_group

additionally i stated in the PR's comment:

It's a very functional solution to aim with sold and bio massed characters. Moving them out of a user group to prevent MissingRefreshToken Exceptions (relevant for SeAT-Groups and Discord-Connector)

As for administration:

Regarding the biomassed character: CCP is writing an email to the whole corporation if someone biomasses the character. As superuser i’d ask the user to provide me some prove before i’d deactivate the user and add that proof to the intel note.

Regarding the selling use-case my recruiter would do exactly the same: some screenshot or the evemail proofing that it has been sold and to whom it was sold. I’d write that to the intel note.


User-Acceptance:

This feature is based from various feedback from my recruiters. We have discussed this in both support and dev channel and my proposal was generally receipted wildly positively.

f.e.
image


The amount of discussion and misconception (f.e. thinking that this is a banning feature or that a user could administrate (selling/biomassing) himself) of it has taken a lot of energy of me and i hope this comment finally resolves this topic.

herpaderpaldent and others added some commits Dec 22, 2018

[fix](ACL): inverse character permission on yourself denys access to …
…own data (#253)

* Fix inverse character permission on associated characters causing to lose
access to own data.

* Apply fixes from StyleCI

* reuse local variable
fix(users): Reduce user coupling and avoid null exception (#251)
* fix(users): Reduce user coupling and avoid null exception

When an user is attached to a non existing group, the user list may
crash due to some relation which are returning a null value.

This is
reducing coupling for an User model by using directly the group_id from
the user entry and excluding things attached to group when none are
returned (mails, role, etc...).

Closes eveseat/seat#491

* fix(users): Fix some null exception related to orphan users in views

Ensure at both login and impersonate that an user is attached to an
existing group. If it's not the case, we're spawning it and update
main character.

Closes eveseat/seat#491

herpaderpaldent and others added some commits Jan 9, 2019

feat(extractions): Design an UI to retrieve pending and active moon-m…
…ining extractions (#262)

* feat(extractions): Design an UI to retrieve pending and active moon-mining extractions

* refactor(extractions): Start to apply some change according to review

* refactor(extractions): Add validator class

* feat(extractions): Switch data collect to service

* feat(extractions): Improve caption using translations
Issue 453 searchblade improvements (#270)
* some minor refactoring and testing of: eveseat/seat#453

* sorting on a column is not triggering the id-to-name resolver;
  which mean corporation stay unresolved
* Using generic partial blades

* some minor refactoring

* Using generic partial blades
* disable columns for search which are irrelevant

* some minor refactoring and testing of eveseat/seat#453

* Using generic partial blades
* disable columns for search which are irrelevant
* disable front end ordering of table as ordering is done in the query.

* some minor refactoring and testing of assets
eveseat/seat#453

* Using generic partial blades
* some minor refactoring and testing of skill search
eveseat/seat#453

* Using generic partial blades
* Add modal to mail-view. Remove redundant partials
* Add custom filter for recipients and from
* strip html tags

@warlof warlof modified the milestones: 3.0.14, 3.0.16 Jan 19, 2019

@warlof warlof removed this from the 3.0.16 milestone Feb 3, 2019

warlof added some commits Feb 15, 2019

3.0.x (#283)
* fix(contracts): Remove non-existing variable from contract index (#272)

* Fix corp killmails table header missarrangement (#275)

* Fix eveseat/seat#509

* Use correct order of headers and data:

# date
# ship
# location
# victim
# zKB link
3.0.x (#284)
* fix(contracts): Remove non-existing variable from contract index (#272)

* Fix corp killmails table header missarrangement (#275)

* Fix eveseat/seat#509

* Use correct order of headers and data:

# date
# ship
# location
# victim
# zKB link

* Fix issue 512 related to corp tracking (#276)

* Add warning about outdated information.

* Fix Issue 512 - Corp Tracking shows people who are not in Corporation
* Don't show non-corp members in member-tracking
* Remove checkbox
* Introduce tabs: Valid, Invalid and Missing on Seat

* Revert "Add warning about outdated information."

This reverts commit 0fdc734

* Implement review feedback

* Used wrong attribute for filterColumn (#277)

* Fix character contracts (#279)

* change `is_nan` to `is_null`

* Don't be to precise

* fix filter column

leonjza and others added some commits Feb 18, 2019

Add warning about outdated information. (#273)
* Add warning about outdated information.

* update text

Update text for warning according to qu1ckkk's inout

@warlof warlof force-pushed the eveseat:master branch 3 times, most recently from 2a1298e to 78ca6a4 Mar 9, 2019

@herpaderpaldent

This comment has been minimized.

Copy link
Contributor Author

commented Apr 14, 2019

This pull request is open from the 8th of november. Your PR-Setup sucks @warlof and @leonjza. If one of you chose to be away from the project the other should be able to review and merge changes.

Your behavior in this pull request is a complete refusal to do your job. How can both of you have not found time to approve/merge this change, i can not comprehend.

Will not further work on this. Feel free to reopen it when you change your setup

@herpaderpaldent herpaderpaldent deleted the herpaderpaldent:449-reassign-user branch Apr 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.