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

✨♻️ webserver: refactored groups plugin and new user privacy compliance #6917

Merged
merged 85 commits into from
Dec 10, 2024

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Dec 6, 2024

What do these changes do?

ReDoc

This pr includes a major refactoring and new logic regarding privacy settings for the groups plugin in the web-server

♻️ Refactoring Highlights

  • Introduced a CSR-layered architecture across subdomains:
    • Groups: Core group operations.
    • Classifiers: Handles group classification.
  • Clear separation of schemas (input/output in REST) and models (internal/domain logic):
    • Conversion between schemas and models implemented as member functions (to_model/from_model) in the schema class.
  • Centralized common functionalities:
    • Exception Handling
    • Unified Models
  • Migrated database to asyncpg for better performance.
  • Consolidated and reduced fixtures, improving test modularity via pytest_simcore.simcore_webserver_groups_fixtures.
  • Enhanced and modernized test structure.

✨ Privacy and Feature Updates

  • Privacy Respect: Group member profiles now display limited information based on user privacy settings.
  • Email-based Addition: Users can only be added by email if privacy settings allow it.
    image

🛠️ Additional Updates

Related issue/s

How to test

Driving tests

cd services/web/server
make install-dev
pytest -vv tests/unit/**/test_*group*.py

Dev-ops

NOne

@pcrespov pcrespov self-assigned this Dec 6, 2024
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 78.26087% with 130 lines in your changes missing coverage. Please review.

Project coverage is 88.12%. Comparing base (561985c) to head (e4a9810).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6917      +/-   ##
==========================================
- Coverage   88.23%   88.12%   -0.11%     
==========================================
  Files        1579     1577       -2     
  Lines       61879    61796      -83     
  Branches     2002     2005       +3     
==========================================
- Hits        54596    54460     -136     
- Misses       6948     7000      +52     
- Partials      335      336       +1     
Flag Coverage Δ
integrationtests 64.97% <51.96%> (-0.13%) ⬇️
unittests 86.39% <77.92%> (-0.05%) ⬇️
Components Coverage Δ
api 76.84% <ø> (ø)
pkg_aws_library 93.49% <ø> (ø)
pkg_dask_task_models_library 97.09% <ø> (ø)
pkg_models_library 91.07% <90.66%> (-0.05%) ⬇️
pkg_notifications_library 84.57% <ø> (ø)
pkg_postgres_database 88.05% <100.00%> (+0.04%) ⬆️
pkg_service_integration 70.02% <ø> (ø)
pkg_service_library 75.52% <ø> (-0.04%) ⬇️
pkg_settings_library 90.60% <ø> (ø)
pkg_simcore_sdk 85.38% <ø> (ø)
agent 97.00% <ø> (ø)
api_server 90.04% <100.00%> (ø)
autoscaling 95.21% <ø> (ø)
catalog 90.57% <ø> (ø)
clusters_keeper 99.48% <ø> (ø)
dask_sidecar 91.26% <ø> (ø)
datcore_adapter 93.18% <ø> (ø)
director 76.49% <ø> (+0.08%) ⬆️
director_v2 91.38% <ø> (ø)
dynamic_scheduler 97.14% <ø> (ø)
dynamic_sidecar 89.75% <ø> (ø)
efs_guardian 90.12% <ø> (ø)
invitations 93.44% <ø> (ø)
osparc_gateway_server ∅ <ø> (∅)
payments 92.65% <ø> (ø)
resource_usage_tracker 89.31% <ø> (+0.07%) ⬆️
storage 89.60% <ø> (ø)
webclient ∅ <ø> (∅)
webserver 87.69% <75.78%> (-0.35%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 561985c...e4a9810. Read the comment docs.

@pcrespov pcrespov added this to the Event Horizon milestone Dec 9, 2024
@pcrespov pcrespov added the a:webserver issue related to the webserver service label Dec 9, 2024
@pcrespov pcrespov force-pushed the is779/prep-users-in-groups branch 2 times, most recently from 2fb361b to ecbc0a3 Compare December 10, 2024 10:07
@pcrespov pcrespov changed the title WIP: Is779/prep users in groups "✨♻️ webserver: refactored groups plugin and new user privacy compliance Dec 10, 2024
@pcrespov pcrespov requested a review from odeimaiz December 10, 2024 10:46
@pcrespov pcrespov marked this pull request as ready for review December 10, 2024 10:46
@pcrespov pcrespov changed the title "✨♻️ webserver: refactored groups plugin and new user privacy compliance ✨♻️ webserver: refactored groups plugin and new user privacy compliance Dec 10, 2024
@odeimaiz
Copy link
Member

:alarm: This will break the frontend. Don't merge it before we release to staging.

Users might not want to be reached (added to a group) by email, but will always be able to be added by their username.

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

just a bit concerned about the organization. Otherwise all good thanks

@pcrespov
Copy link
Member Author

:alarm: This will break the frontend. Don't merge it before we release to staging.

Users might not want to be reached (added to a group) by email, but will always be able to be added by their username.

@odeimaiz
image

Copy link
Contributor

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for refactoring!

@matusdrobuliak66
Copy link
Contributor

Q: Is it still valid, that organizations are groups with read access True?

@pcrespov pcrespov force-pushed the is779/prep-users-in-groups branch from 3e9b78b to eb6071d Compare December 10, 2024 15:13
Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

👌

@pcrespov
Copy link
Member Author

pcrespov commented Dec 10, 2024

Q: Is it still valid, that organizations are groups with read access True?

@matusdrobuliak66 I limited the REST api to do write/delete operations to standard groups. In this sense, "organization" is a front-end concept. Those are standard-groups owned by a user and therefore it can operate on them.

The groups associated to a product are indeed standard groups but w/o any access to the users. Therefore these are never provided via the REST api except for a small section in get_all_my_groups in which are passed to the front-end

@pcrespov pcrespov enabled auto-merge (squash) December 10, 2024 20:56
@pcrespov pcrespov disabled auto-merge December 10, 2024 21:57
@pcrespov pcrespov merged commit 3960405 into ITISFoundation:master Dec 10, 2024
89 of 93 checks passed
@pcrespov pcrespov deleted the is779/prep-users-in-groups branch December 10, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants