fedora-infra / fas

Fedora Account System
https://admin.fedoraproject.org/accounts
GNU General Public License v2.0
40 stars 50 forks source link

Delete group crashes on the admin panel #258

Closed ryanlerch closed 3 years ago

ryanlerch commented 7 years ago

trying to delete a group on the admin panel doenst work, and the follow error is given to the console:

2017-02-02 11:41:19,092 ERROR [waitress:1794][waitress:channel][service:341] Exception when serving /settings/remove/group/20000
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/waitress/channel.py", line 338, in service
    task.service()
  File "/usr/lib/python2.7/site-packages/waitress/task.py", line 169, in service
    self.execute()
  File "/usr/lib/python2.7/site-packages/waitress/task.py", line 399, in execute
    app_iter = self.channel.server.application(env, start_response)
  File "/usr/lib/python2.7/site-packages/pyramid/router.py", line 233, in __call__
    response = self.invoke_subrequest(request, use_tweens=True)
  File "/usr/lib/python2.7/site-packages/pyramid/router.py", line 208, in invoke_subrequest
    response = handle_request(request)
  File "/usr/lib/python2.7/site-packages/pyramid_debugtoolbar/toolbar.py", line 187, in toolbar_tween
    return handler(request)
  File "/usr/lib/python2.7/site-packages/pyramid/tweens.py", line 62, in excview_tween
    reraise(*attrs['exc_info'])
  File "/usr/lib/python2.7/site-packages/pyramid/tweens.py", line 22, in excview_tween
    response = handler(request)
  File "/usr/lib/python2.7/site-packages/pyramid_tm/__init__.py", line 119, in tm_tween
    reraise(*exc_info)
  File "/usr/lib/python2.7/site-packages/pyramid_tm/__init__.py", line 98, in tm_tween
    response = handler(request)
  File "/usr/lib/python2.7/site-packages/pyramid/router.py", line 155, in handle_request
    view_name
  File "/usr/lib/python2.7/site-packages/pyramid/view.py", line 612, in _call_view
    response = view_callable(context, request)
  File "/usr/lib/python2.7/site-packages/pyramid/config/views.py", line 181, in __call__
    return view(context, request)
  File "/usr/lib/python2.7/site-packages/pyramid/viewderivers.py", line 389, in attr_view
    return view(context, request)
  File "/usr/lib/python2.7/site-packages/pyramid/viewderivers.py", line 367, in predicate_wrapper
    return view(context, request)
  File "/usr/lib/python2.7/site-packages/pyramid/viewderivers.py", line 300, in secured_view
    return view(context, request)
  File "/usr/lib/python2.7/site-packages/pyramid/viewderivers.py", line 438, in rendered_view
    result = view(context, request)
  File "/usr/lib/python2.7/site-packages/pyramid/viewderivers.py", line 123, in _class_requestonly_view
    response = getattr(inst, attr)()
  File "/vagrant/fas/views/admin.py", line 267, in remove_group
    register.remove_group(group)
  File "/vagrant/fas/models/register.py", line 305, in remove_group
    session.flush()
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/orm/scoping.py", line 157, in do
    return getattr(self.registry(), name)(*args, **kwargs)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/orm/session.py", line 2139, in flush
    self._flush(objects)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/orm/session.py", line 2259, in _flush
    transaction.rollback(_capture_exception=True)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/util/langhelpers.py", line 60, in __exit__
    compat.reraise(exc_type, exc_value, exc_tb)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/orm/session.py", line 2223, in _flush
    flush_context.execute()
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/orm/unitofwork.py", line 384, in execute
    n.execute_aggregate(self, set_)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/orm/unitofwork.py", line 481, in execute_aggregate
    self.execute(uow)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/orm/unitofwork.py", line 548, in execute
    uow
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/orm/persistence.py", line 177, in save_obj
    mapper, table, update)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/orm/persistence.py", line 737, in _emit_update_statements
    execute(statement, multiparams)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/base.py", line 945, in execute
    return meth(self, multiparams, params)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/sql/elements.py", line 263, in _execute_on_connection
    return connection._execute_clauseelement(self, multiparams, params)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/base.py", line 1053, in _execute_clauseelement
    compiled_sql, distilled_params
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/base.py", line 1189, in _execute_context
    context)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/base.py", line 1393, in _handle_dbapi_exception
    exc_info
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/util/compat.py", line 203, in raise_from_cause
    reraise(type(exception), exception, tb=exc_tb, cause=cause)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/base.py", line 1182, in _execute_context
    context)
  File "/usr/lib64/python2.7/site-packages/sqlalchemy/engine/default.py", line 470, in do_execute
    cursor.execute(statement, parameters)
IntegrityError: (psycopg2.IntegrityError) null value in column "group_id" violates not-null constraint
DETAIL:  Failing row contains (6562, null, 4, 1, null, 7, null, 2017-02-02 11:17:36.976682, 2017-02-02 11:41:18.983211).
 [SQL: 'UPDATE group_membership SET group_id=%(group_id)s, update_timestamp=CURRENT_TIMESTAMP WHERE group_membership.id = %(group_membership_id)s'] [parameters: {'group_id': None, 'group_membership_id': 6562}]
laxathom commented 7 years ago

Since you assigned the issue to yourself, there are a few thing to look at.

The current workflow to remove a group is to, first remove all related membership by hand but you probably noticed that, which leads me to say that you should not get this kind of error but a pop-up asking you to revoke all membership first.

This is the current behavior I have with current HEAD.

Since then, we introduced "cascade" mechanism into the data models which take care of the membership removal when a group is removed (The issue you're having looks more related to a cascade delete issue). However, we did not remove the group membership check which prevents group's removal unless you did it.

Even though if you revoke all membership, you will hit the behavior explained above as this actually raise another issue which is: membership don't get deleted but archived in a kind of way (by setting member's status to UNAPPROVED as fedoraproject doesn't remove anything but this should not be done at a code-base).

Looking at the current HEAD, the cascade option looks good and should remove all related membership if the membership check mechanism is removed unless your data models is not up-to-date into postgres.

By fixing this issue, we should update group removal workflow. What I would suggest is to pop-up a dialog to ask confirmation of the delete. If admin confirms, delete the group and send out delete's notification to all former members. Notification should already be handled by the removal notification's mechanism.

ryanlerch commented 3 years ago

Closing this issue as the FAS project is now archived, not actively developed, and unmaintained.

FAS was replaced in March 2021 by Fedora Accounts (https://accounts.fedoraproject.org).

If this issue is a Feature Request that you forsee might be beneficial to Fedora Accounts, please refile it against Noggin