Skip to content

Commit

Permalink
frontend: group alias need 1:1 mapping to group
Browse files Browse the repository at this point in the history
When i was developing group supported in OIDC, i found some wired hehavior in group activated in #2825,
So i submitted this PR, and @praiskup also give advice a TODO so i include the fix as well

This PR try to fix: #2825
    1. add unique contraint to fas_name and add migration script generated by alembic revision --autogenerate
    2. I think macro fas_group_href canbe replaced by config.GROUP_INFO.link.forma which is more configurable
    3. When creating a new alias for a group, if the group already has one alias, popup a flash and redirect to the already existed group

Signed-off-by: Li Chaoran <[email protected]>
  • Loading branch information
pkking committed Aug 1, 2023
1 parent 575dbe3 commit 5647ba1
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
"""
add unique constraint to fas_group
Revision ID: daa62cd0743d
Revises: ba6ac0936bfb
Create Date: 2023-08-01 09:52:01.522171
"""

from alembic import op


# revision identifiers, used by Alembic.
revision = 'daa62cd0743d'
down_revision = 'ba6ac0936bfb'
branch_labels = None
depends_on = None


def upgrade():
op.create_unique_constraint(None, 'group', ['fas_name'])


def downgrade():
op.drop_constraint(None, 'group', type_='unique')
3 changes: 1 addition & 2 deletions frontend/coprs_frontend/coprs/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2206,8 +2206,7 @@ class Group(db.Model, helpers.Serializer):
id = db.Column(db.Integer, primary_key=True)
name = db.Column(db.String(127))

# TODO: add unique=True
fas_name = db.Column(db.String(127))
fas_name = db.Column(db.String(127), unique=True)

@property
def at_name(self):
Expand Down
7 changes: 1 addition & 6 deletions frontend/coprs_frontend/coprs/templates/_helpers.html
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ <h3 class="panel-title"> {{ g.user.name | capitalize}} </h3>
<span class="badge">{{ g.user.coprs_count }}</span>
My projects
</a>
{% if config.FAS_LOGIN or config.LDAP_URL %}
{% if config.FAS_LOGIN or config.LDAP_URL or config.OIDC_LOGIN %}
<a href="{{url_for('groups_ns.list_user_groups') }}" class="list-group-item">
<span class="badge"> {{ user.user_groups|length }} </span>
My groups
Expand Down Expand Up @@ -458,11 +458,6 @@ <h3 class="panel-title">News - <a href="{{ config.NEWS_URL }}">Read All</a></h3>
{{- copr_url('coprs_ns.copr_package', package.copr, package_name=package.name) -}}
{% endmacro %}

{%- macro fas_group_href(name) -%}
https://accounts.fedoraproject.org/group/{{name}}
{%- endmacro -%}


{% macro repo_file_href(copr, repo, arch=None) %}
{%- if not arch %}
{{- owner_url('coprs_ns.generate_repo_file',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{% extends "coprs/group_show.html" %}
{% block title %}Project List{% endblock %}
{% block header %}Project List{% endblock %}
{% from "_helpers.html" import render_pagination, fas_group_href %}
{% from "_helpers.html" import render_pagination %}
{% block breadcrumbs %}
<ol class="breadcrumb">
<li>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{% extends "layout.html" %}
{% from "_helpers.html" import fas_group_href, initialize_datatables %}
{% from "_helpers.html" import initialize_datatables %}
{% block title %}Add a Group{% endblock %}
{% block header %}Add a Group{% endblock %}

Expand Down Expand Up @@ -37,7 +37,7 @@ <h1>
<tbody>
{% for team in teams %}
<tr>
<td><a href="{{ fas_group_href(team) }}">
<td><a href="{{ config.GROUP_INFO.link.format(name=team) }}">
{{ team }}
</a></td>
<td>
Expand Down
19 changes: 12 additions & 7 deletions frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from coprs.logic.coprs_logic import CoprsLogic, PinnedCoprsLogic
from coprs.logic.users_logic import UsersLogic
from coprs import app
from coprs.oidc import oidc_enabled

from ... import db
from ..misc import login_required
Expand All @@ -31,7 +32,7 @@ def activate_group(fas_group):

if fas_group not in flask.g.user.user_teams:
raise InsufficientRightsException(
"User '{}' doesn't have access to fas group {}"
"User '{}' doesn't have access to group {}"
.format(flask.g.user.username, fas_group))

alias = form.name.data
Expand All @@ -41,12 +42,16 @@ def activate_group(fas_group):
db.session.add(group)
db.session.commit()

flask.flash(
"FAS group {} is activated in the Copr under the alias {} "
.format(fas_group, alias)
)
# can't have more than one alias for a group
if group.name != alias:
message = "Group {} already activated as alias {} ".format(
fas_group, group.name)
else:
message = "Group {} is activated in the System under the alias {} ".format(
fas_group, alias)
flask.flash(message)
return flask.redirect(url_for(
"groups_ns.list_projects_by_group", group_name=alias))
"groups_ns.list_projects_by_group", group_name=group.name))

else:
return flask.render_template(
Expand Down Expand Up @@ -85,7 +90,7 @@ def list_projects_by_group(group_name, page=1):
@groups_ns.route("/list/my")
@login_required
def list_user_groups():
if not (app.config['FAS_LOGIN'] or app.config['LDAP_URL']):
if not (app.config['FAS_LOGIN'] or app.config['LDAP_URL'] or oidc_enabled(app.config)):
raise ObjectNotFound("Fedora Accounts or LDAP groups not enabled")

teams = flask.g.user.user_teams
Expand Down

0 comments on commit 5647ba1

Please sign in to comment.