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

LITE-28161 Add user id and name to Configuration events #42

Merged
merged 1 commit into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions connect_ext_ppr/models/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ class Configuration(Model):
default=ConfigurationStateChoices.INACTIVE,
)
created_at = db.Column(db.DateTime(), default=datetime.utcnow)
created_by = db.Column(db.String(20))
created_by = db.Column(db.JSON)
updated_at = db.Column(db.DateTime(), default=datetime.utcnow)
updated_by = db.Column(db.String(20))
updated_by = db.Column(db.JSON)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly speaking, I don't like the idea of storing it as JSON here, cause of:

  1. What id the name of the user changed?
  2. Filtering, rql or other filters doesn't support JSON fields filters for now

Let's have it as is for now, but I'll put it as the gap


def activate(self):
self.state = ConfigurationStateChoices.ACTIVE
Expand Down
2 changes: 1 addition & 1 deletion connect_ext_ppr/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class ConfigurationSchema(NonNullSchema):
id: str
file: FileSchema
state: ConfigurationStateChoices
events: Dict[str, Dict[str, Union[datetime, str]]]
events: Dict[str, Dict[str, Union[datetime, Dict[str, str]]]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the future --> TypeAliases will help you



class ConfigurationReferenceSchema(NonNullSchema):
Expand Down
20 changes: 18 additions & 2 deletions connect_ext_ppr/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from connect.eaas.core.logging import RequestLogger
from fastapi import status
from jsonschema.exceptions import _Error
import jwt
import pandas as pd

from connect_ext_ppr.errors import ExtensionHttpError
Expand Down Expand Up @@ -224,8 +225,14 @@ def get_configuration_schema(configuration, file):
file=file_schema,
state=configuration.state,
events={
'created': {'at': configuration.created_at},
'updated': {'at': configuration.updated_at},
'created': {
'at': configuration.created_at,
'by': configuration.created_by,
},
'updated': {
'at': configuration.updated_at,
'by': configuration.updated_by,
},
},
)

Expand Down Expand Up @@ -279,3 +286,12 @@ def _parse_json_schema_error(ex: _Error):
sub_list = _parse_json_schema_error(sub_ex)
error_list.extend(sub_list)
return error_list


def get_user_data_from_auth_token(token):
payload = jwt.decode(token, options={"verify_signature": False})

return {
'id': payload['u']['oid'],
'name': payload['u']['name'],
}
11 changes: 8 additions & 3 deletions connect_ext_ppr/webapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
)
from connect.eaas.core.inject.synchronous import get_installation, get_installation_client
from connect.eaas.core.extension import WebApplicationBase
from fastapi import Depends, Response, status
from fastapi import Depends, Request, Response, status
from sqlalchemy import exists
from sqlalchemy.exc import SQLAlchemyError
from sqlalchemy.orm import joinedload
Expand Down Expand Up @@ -44,6 +44,7 @@
get_deployment_request_schema,
get_deployment_schema,
get_hubs,
get_user_data_from_auth_token,
)


Expand All @@ -61,7 +62,7 @@ class ConnectExtensionXvsWebApplication(WebApplicationBase):

@router.get(
'/deployments/requests',
summary='List all request accross deployments',
summary='List all requests across deployments',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just typos

response_model=List[DeploymentRequestSchema],
)
def list_deployment_requests(
Expand Down Expand Up @@ -220,6 +221,7 @@ def add_configuration(
deployment_id: str,
db: VerboseBaseSession = Depends(get_db),
installation: dict = Depends(get_installation),
request: Request = None,
):
get_deployment_by_id(deployment_id, db, installation)
file_data = configuration.file
Expand All @@ -238,7 +240,7 @@ def add_configuration(
mime_type=file_data.mime_type,
)
db.add(file_instance)
db.commit()
db.flush()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed for transaction because db.commit() ends transaction


prev_configuration = (
db.query(Configuration)
Expand All @@ -251,10 +253,13 @@ def add_configuration(
if prev_configuration:
prev_configuration.state = ConfigurationStateChoices.INACTIVE

user_data = get_user_data_from_auth_token(request.headers['connect-auth'])
configuration_instance = Configuration(
file=file_data.id,
deployment=deployment_id,
state=ConfigurationStateChoices.ACTIVE,
created_by=user_data,
updated_by=user_data,
)
db.set_verbose(configuration_instance)
db.commit()
Expand Down
19 changes: 18 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ sqlalchemy = "^1.3.12"
psycopg2-binary = "^2.9.6"
pandas = "^2.0.3"
openpyxl = "^3.1.2"
PyJWT = "^2.8.0"

[tool.poetry.dev-dependencies]
pytest = ">=6.1.2,<8"
Expand Down
21 changes: 20 additions & 1 deletion tests/api/test_configurations.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ def test_post_configuration(
'mime_type': media_response['mime_type'],
},
},
headers={
"connect-auth": (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For tests it is better to generate the test signature based on human-readable input. Otherwise hard to understand what is inside and what should be expected in the tests

"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1Ijp7Im9pZCI6IlNVLTI5NS02ODktN"
"jI4IiwibmFtZSI6Ik5lcmkifX0.U_T6vuXnD293hcWNTJZ9QBViteNv8JXUL2gM0BezQ-k"
),
},
)
assert response.status_code == 200
data = response.json()
Expand All @@ -123,8 +129,15 @@ def test_post_configuration(
'size': 17,
'mime_type': 'application/json',
}

assert data['state'] == ConfigurationStateChoices.ACTIVE
assert data['events']['created']['by'] == {
'id': 'SU-295-689-628',
'name': 'Neri',
}
assert data['events']['updated']['by'] == {
'id': 'SU-295-689-628',
'name': 'Neri',
}

assert dbsession.query(Configuration).count() == 1
assert dbsession.query(File).count() == 1
Expand All @@ -150,6 +163,12 @@ def test_post_configuration_deactivate_previous(
'mime_type': file.mime_type,
},
},
headers={
"connect-auth": (
"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1Ijp7Im9pZCI6IlNVLTI5NS02ODktN"
"jI4IiwibmFtZSI6Ik5lcmkifX0.U_T6vuXnD293hcWNTJZ9QBViteNv8JXUL2gM0BezQ-k"
),
},
)
assert response.status_code == 200

Expand Down
12 changes: 11 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,10 +474,12 @@ def api_client(test_client_factory, dbsession):


@pytest.fixture
def configuration(dbsession, deployment, file):
def configuration(dbsession, deployment, file, user):
conf = Configuration(
file=file.id,
deployment=deployment.id,
created_by=user,
updated_by=user,
)
dbsession.set_verbose(conf)
dbsession.commit()
Expand Down Expand Up @@ -779,3 +781,11 @@ def price_proposal_response():
'currency': [],
},
}


@pytest.fixture
def user():
return {
'id': 'SU-295-689-628',
'name': 'Neri',
}
14 changes: 10 additions & 4 deletions tests/test_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ def test_configuration_schema(state, file):
),
state=state,
events={
'created': {'at': now, 'by': 'SU-295-689-628'},
'updated': {'at': now, 'by': 'SU-295-689-628'},
'created': {'at': now, 'by': {'id': 'SU-295-689-628', 'name': 'Neri'}},
'updated': {'at': now, 'by': {'id': 'SU-295-689-628', 'name': 'Neri'}},
},
)
assert serializer.dict() == {
Expand All @@ -137,11 +137,17 @@ def test_configuration_schema(state, file):
"events": {
"created": {
"at": now,
"by": "SU-295-689-628",
"by": {
'id': 'SU-295-689-628',
'name': 'Neri',
},
},
"updated": {
"at": now,
"by": "SU-295-689-628",
"by": {
'id': 'SU-295-689-628',
'name': 'Neri',
},
},
},
}
Expand Down