Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 21, 2025

What do these changes do?

This pull request refactors user admin API query handling, and adds robust ordering support for API list operations. The main changes include enhanced ordering query parameter models, updates to user account filtering and ordering and a new convention for documenting type annotations. Additionally, invitation models now enforce lowercase emails and use more precise type annotations.

API List Operations and Ordering Support:

  • Introduced OrderDirection, OrderClause, and check_ordering_list in the new list_operations.py module, providing a generic and validated way to handle multi-field ordering for API queries.
  • Refactored rest_ordering.py to support generic ordering via OrderingQueryParams, parsing comma-separated strings into validated order clauses and handling duplicates/conflicts robustly. [1] [2]

User Admin API Query Refactoring:

  • Updated user admin API endpoint to use a new query parameter model (_Q) that combines filtering and pagination, and added support for an order_by parameter for flexible sorting. [1] [2]
  • Refactored UsersAccountListQueryParams to inherit from OrderingQueryParams and define valid ordering fields, improving API consistency and validation. [1] [2]

User Account Search Query Enhancements:

  • Modified glob string constraints and field descriptions to clarify that searches are case-insensitive, improving usability and documentation. [1] [2] [3]

Invitation Model Improvements:

  • Updated invitation models to use Annotated for all fields, enforce lowercase emails via AfterValidator, and clarify docstrings. Also switched to using datetime.now(tz=UTC) for timestamps. [1] [2] [3] [4]

Python Coding Instructions and Documentation:

  • Added a new section to .github/instructions/python.instructions.md describing how to use annotated_types.doc() for parameter and return type documentation, replacing traditional docstring Args/Returns sections. This encourages concise, type-driven documentation and provides usage examples.
  • Updated coding guidelines to clarify when to document parameters and how to use docstrings, and reorganized sections for clarity. [1] [2] [3]

Related issue/s

  • Implementing follow up on ITISFoundation/private-issues#23

How to test

Dev-ops

@pcrespov pcrespov added this to the Imparable milestone Oct 21, 2025
@pcrespov pcrespov self-assigned this Oct 21, 2025
@pcrespov pcrespov added a:webserver webserver's codebase. Assigning the area is particularly useful for bugs a:models-library labels Oct 21, 2025
@codecov
Copy link

codecov bot commented Oct 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.44%. Comparing base (48c7ccc) to head (8fb1d3d).

❗ There is a different number of reports uploaded between BASE (48c7ccc) and HEAD (8fb1d3d). Click for more details.

HEAD has 30 uploads less than BASE
Flag BASE (48c7ccc) HEAD (8fb1d3d)
unittests 31 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #8545       +/-   ##
===========================================
- Coverage   84.72%   66.44%   -18.29%     
===========================================
  Files        2010      804     -1206     
  Lines       78561    36158    -42403     
  Branches     1349      175     -1174     
===========================================
- Hits        66559    24024    -42535     
- Misses      11598    12077      +479     
+ Partials      404       57      -347     
Flag Coverage Δ *Carryforward flag
integrationtests 63.98% <ø> (+3.55%) ⬆️ Carriedforward from 8cc132b
unittests 97.95% <ø> (+11.55%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 76.75% <ø> (-8.21%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter 97.95% <ø> (ø)
director ∅ <ø> (∅)
director_v2 78.11% <ø> (+17.22%) ⬆️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 81.89% <ø> (-8.55%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 58.96% <ø> (-28.14%) ⬇️

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 48c7ccc...8fb1d3d. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pcrespov pcrespov requested a review from Copilot October 21, 2025 17:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors user admin API query handling and adds robust ordering support for API list operations. The main changes include:

  • Introduction of generic ordering infrastructure via OrderingQueryParams with support for comma-separated field ordering
  • Enhanced user account listing with validated multi-field ordering capabilities
  • Improved type annotations using annotated_types.doc() for clearer parameter documentation
  • Email validation improvements in invitation models with lowercase normalization

Reviewed Changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
.github/instructions/python.instructions.md Added guidelines for using annotated_types.doc() for parameter documentation
packages/models-library/src/models_library/list_operations.py New module defining OrderDirection, OrderClause, and check_ordering_list for validated ordering operations
packages/models-library/src/models_library/rest_ordering.py Refactored to add OrderingQueryParams with comma-separated string parsing support
packages/models-library/src/models_library/invitations.py Updated to use Annotated types, enforce lowercase emails, and use datetime.now(tz=UTC)
packages/models-library/src/models_library/api_schemas_webserver/users.py Added UserAccountOrderFields and integrated OrderingQueryParams into UsersAccountListQueryParams
services/web/server/src/simcore_service_webserver/users/_accounts_repository.py Added OrderKeys, MergedUserData types, and ordering support in list_merged_pre_and_registered_users
services/web/server/src/simcore_service_webserver/users/_accounts_service.py Updated documentation using annotated_types.doc() and added order_by parameter support
services/web/server/src/simcore_service_webserver/users/_controller/rest/accounts_rest.py Added field mapping for ordering and integrated order_by into list endpoint
services/web/server/src/simcore_service_webserver/users/plugin.py Renamed import from users_rest to profile_rest
api/specs/web-server/_users_admin.py Updated to use separate query model and explicit order_by parameter
api/specs/web-server/_common.py Improved assertion error message in as_query
Test files Added comprehensive tests for ordering validation, invitation timestamps, and case-insensitive search

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@mergify
Copy link
Contributor

mergify bot commented Oct 21, 2025

🧪 CI Insights

Here's what we observed from your CI run for 8fb1d3d.

❌ Job Failures

Pipeline Job Health on master Retries 🔍 CI Insights 📄 Logs
CI integration-tests Broken 0 View View
unit-tests Healthy 0 View View

…documentation and validation for order_by field
…ype and update ordering logic with OrderDirection
…ype annotations; remove unused functions and imports
…tation and remove unused arguments; enhance user account listing with order mapping
@pcrespov pcrespov force-pushed the is23/po-center-preparation branch from 5ee8462 to 6d80b2c Compare October 21, 2025 17:45
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:models-library a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant