Merge pull request #68 from AustralianBioCommons/remove-bpa-register-orgs

AAI-352: Remove orgs from bpa registration page

Author: minh-biocommons

.gitignore

@@ -174,3 +174,6 @@ cython_debug/
 .pypirc
 
 .vscode/
+
+# Local database file
+database.db

config.py

@@ -1,5 +1,5 @@
 from functools import lru_cache
-from typing import Dict, Optional
+from typing import Optional
 
 from pydantic_settings import BaseSettings, SettingsConfigDict
 
@@ -19,26 +19,6 @@ class Settings(BaseSettings):
     # Note we process this separately in app startup as it needs
     #   to be available before the app starts
     cors_allowed_origins: str
-    organizations: Dict[str, str] = {
-        "bpa-bioinformatics-workshop": "2024 Fungi Bioinformatics Workshop",
-        "cipps": "ARC for Innovations in Peptide and Protein Science (CIPPS)",
-        "ausarg": "Australian Amphibian and Reptile Genomics",
-        "aus-avian": "Australian Avian Genomics",
-        "aus-fish": "Australian Fish Genomics",
-        "grasslands": "Australian Grasslands Initiative",
-        "fungi": "Fungi Functional 'Omics",
-        "forest-resilience": "Genomics for Forest Resilience",
-        "bpa-great-barrier-reef": "Great Barrier Reef",
-        "bpa-ipm": "Integrated Pest Management 'Omics",
-        "bpa-omg": "Oz Mammals Genomics Initiative",
-        "plant-pathogen": "Plant Pathogen 'Omics",
-        "ppa": "Plant Protein Atlas",
-        "australian-microbiome": "The Australian Microbiome Initiative",
-        "threatened-species": "Threatened Species Initiative",
-        "bpa-wheat-cultivars": "Wheat Cultivars",
-        "bpa-wheat-pathogens-genomes": "Wheat Pathogens Genomes",
-        "bpa-wheat-pathogens-transcript": "Wheat Pathogens Transcript",
-    }
 
     model_config = SettingsConfigDict(env_file=".env", extra="ignore")
 

routers/bpa_register.py

@@ -1,12 +1,11 @@
 import logging
 from datetime import datetime, timezone
 
-from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException
+from fastapi import APIRouter, Depends, HTTPException
 from httpx import HTTPStatusError
 from sqlmodel import Session
 from starlette.responses import JSONResponse
 
-from auth.ses import EmailService
 from auth0.client import Auth0Client, get_auth0_client
 from config import Settings, get_settings
 from db.models import BiocommonsUser, PlatformEnum
@@ -15,7 +14,7 @@
 from schemas.biocommons import Auth0UserData, BiocommonsRegisterData
 from schemas.bpa import BPARegistrationRequest
 from schemas.responses import RegistrationErrorResponse, RegistrationResponse
-from schemas.service import Resource, Service
+from schemas.service import Service
 
 logger = logging.getLogger(__name__)
 
@@ -27,57 +26,14 @@
 )
 
 
-def send_approval_email(registration: BPARegistrationRequest, bpa_resources: list[Resource]):
-    email_service = EmailService()
-    approver_email = "aai-dev@biocommons.org.au"
-    subject = "New BPA User Access Request"
-
-    org_list_html = "".join(
-        f"<li>{res.name} (ID: {res.id})</li>" for res in bpa_resources
-    )
-
-    body_html = f"""
-        <p>A new user has requested access to one or more organizations in the BPA service.</p>
-        <p><strong>User:</strong> {registration.fullname} ({registration.email})</p>
-        <p><strong>Requested access to:</strong></p>
-        <ul>{org_list_html}</ul>
-        <p>Please <a href='https://aaiportal.test.biocommons.org.au/requests'>log into the AAI Admin Portal</a> to review and approve access.</p>
-    """
-
-    email_service.send(approver_email, subject, body_html)
-
-
-def _get_bpa_resources(registration: BPARegistrationRequest, settings: Settings, update_time: datetime) -> list[Resource]:
-    bpa_resources = []
-    for org_id, is_selected in registration.organizations.items():
-        if not is_selected:
-            continue
-        if org_id not in settings.organizations:
-            raise HTTPException(
-                status_code=400, detail=f"Invalid organization ID: {org_id}"
-            )
-        resource = Resource(
-            id=org_id,
-            name=settings.organizations[org_id],
-            status="pending",
-            last_updated=update_time,
-            initial_request_time=update_time,
-            updated_by="system",
-        ).model_dump(mode="json")
-        bpa_resources.append(resource)
-    return bpa_resources
-
-
 def _get_bpa_service_request(registration: BPARegistrationRequest, settings: Settings, update_time: datetime) -> Service:
-    bpa_resources = _get_bpa_resources(registration, settings, update_time)
     return Service(
         name="Bioplatforms Australia Data Portal",
         id="bpa",
         initial_request_time=update_time,
         status="pending",
         last_updated=update_time,
         updated_by="system",
-        resources=bpa_resources,
     )
 
 
@@ -90,12 +46,11 @@ def _get_bpa_service_request(registration: BPARegistrationRequest, settings: Set
 )
 async def register_bpa_user(
     registration: BPARegistrationRequest,
-    background_tasks: BackgroundTasks,
     settings: Settings = Depends(get_settings),
     db_session: Session = Depends(get_db_session),
     auth0_client: Auth0Client = Depends(get_auth0_client)
 ):
-    """Register a new BPA user with selected organization resources."""
+    """Register a new BPA user."""
     now = datetime.now(timezone.utc)
     bpa_service = _get_bpa_service_request(registration=registration, settings=settings, update_time=now)
 
@@ -111,9 +66,6 @@ async def register_bpa_user(
         logger.info("Adding user to DB")
         _create_bpa_user_record(auth0_user_data, db_session)
 
-        if bpa_service.resources and settings.send_email:
-            background_tasks.add_task(send_approval_email, registration, bpa_resources=bpa_service.resources)
-
         return {"message": "User registered successfully", "user": auth0_user_data.model_dump(mode="json")}
 
     # Return HTTP status errors as RegistrationErrorResponse

schemas/bpa.py

@@ -1,5 +1,3 @@
-from typing import Dict
-
 from pydantic import BaseModel, EmailStr
 
 from schemas.biocommons import BiocommonsPassword, BiocommonsUsername
@@ -11,4 +9,3 @@ class BPARegistrationRequest(BaseModel):
     email: EmailStr
     reason: str
     password: BiocommonsPassword
-    organizations: Dict[str, bool]

tests/datagen.py

@@ -109,15 +109,6 @@ def confirmPassword(cls, password: str) -> str:
 class BPARegistrationDataFactory(ModelFactory[BPARegistrationRequest]):
     """Factory for generating BPA registration test data."""
 
-    @classmethod
-    def get_default_organizations(cls) -> dict:
-        """Default organization selection."""
-        return {
-            "bpa-bioinformatics-workshop": True,
-            "cipps": False,
-            "ausarg": True,
-        }
-
     password = BiocommonsProviders.biocommons_password
     username = BiocommonsProviders.biocommons_username
 

tests/test_bpa_register.py

@@ -28,7 +28,6 @@ def valid_registration_data():
         email="test@example.com",
         reason="Need access to BPA resources",
         password="SecurePass123!",
-        organizations=BPARegistrationDataFactory.get_default_organizations(),
     ).model_dump()
 
 
@@ -50,22 +49,17 @@ def test_to_biocommons_register_data(valid_registration_data):
 
 
 def test_successful_registration(
-    test_client_with_email, mocker, valid_registration_data,
-        mock_auth0_client, test_db_session
+    test_client, valid_registration_data, mock_auth0_client, test_db_session
 ):
     """Test successful user registration with BPA service"""
-    test_client = test_client_with_email
     user_id = random_auth0_id()
     mock_auth0_client.create_user.return_value = Auth0UserDataFactory.build(user_id=user_id)
-    mock_email_cls = mocker.patch("routers.bpa_register.EmailService", autospec=True)
-    mock_email_cls.return_value.send.return_value = None
 
     response = test_client.post("/bpa/register", json=valid_registration_data)
 
     assert response.status_code == 200
     assert response.json()["message"] == "User registered successfully"
 
-    mock_email_cls.return_value.send.assert_called_once()
     # Check user is created in the database
     db_user = test_db_session.get(BiocommonsUser, user_id)
     assert db_user is not None
@@ -95,12 +89,7 @@ def test_successful_registration(
     assert bpa_service.status == "pending"
     assert bpa_service.last_updated is not None
     assert bpa_service.updated_by == "system"
-    assert len(bpa_service.resources) == 2
-
-    for resource in bpa_service.resources:
-        assert resource.last_updated is not None
-        assert resource.initial_request_time is not None
-        assert resource.updated_by == "system"
+    assert len(bpa_service.resources) == 0
 
     assert (
         called_data.user_metadata.bpa.registration_reason
@@ -165,25 +154,11 @@ def test_registration_auth0_error(
     assert response.json()["message"] == "Auth0 error: Something went wrong"
 
 
-def test_registration_with_invalid_organization(
-    test_client, valid_registration_data
-):
-    """Test registration with invalid organization ID"""
-    data = valid_registration_data.copy()
-    data["organizations"] = {"invalid-org-id": True}
-
-    response = test_client.post("/bpa/register", json=data)
-
-    assert response.status_code == 400
-    assert "Invalid organization ID" in response.json()["detail"]
-
-
 def test_registration_request_validation(test_client):
     """Test request validation"""
     invalid_data = {
         "username": "testuser",
         "email": "invalid-email",
-        "organizations": {},
     }
 
     response = test_client.post("/bpa/register", json=invalid_data)
@@ -194,45 +169,6 @@ def test_registration_request_validation(test_client):
     assert any(error["field"] == "email" for error in error_data["field_errors"])
 
 
-def test_no_selected_organizations(
-    test_client, test_db_session, mock_auth0_client, valid_registration_data
-):
-    """Test registration with no organizations selected"""
-    data = valid_registration_data.copy()
-    data["organizations"] = {
-        "bpa-bioinformatics-workshop": False,
-        "cipps": False,
-        "ausarg": False,
-    }
-    user_data = Auth0UserDataFactory.build()
-    mock_auth0_client.create_user.return_value = user_data
-
-    response = test_client.post("/bpa/register", json=data)
-
-    assert response.status_code == 200
-    # Check user data sent to Auth0
-    called_data = mock_auth0_client.create_user.call_args[0][0]
-    bpa_service = called_data.app_metadata.services[0]
-    assert len(bpa_service.resources) == 0
-
-
-def test_empty_organizations_dict(
-    test_client, test_db_session, mock_auth0_client, valid_registration_data
-):
-    """Test registration with empty organizations dictionary"""
-    data = valid_registration_data.copy()
-    data["organizations"] = {}
-    user_data = Auth0UserDataFactory.build()
-    mock_auth0_client.create_user.return_value = user_data
-
-    response = test_client.post("/bpa/register", json=data)
-
-    assert response.status_code == 200
-    called_data = mock_auth0_client.create_user.call_args[0][0]
-    bpa_service = called_data.app_metadata.services[0]
-    assert len(bpa_service.resources) == 0
-
-
 def test_registration_email_format(test_client, valid_registration_data):
     """Test email format validation"""
     data = valid_registration_data.copy()
@@ -244,31 +180,3 @@ def test_registration_email_format(test_client, valid_registration_data):
     details = response.json()
     errors = details["field_errors"]
     assert "email" in [error["field"] for error in errors]
-
-
-def test_all_organizations_selected(
-    test_client_with_email,
-    test_db_session,
-    mock_settings,
-    mocker,
-    mock_auth0_client,
-    valid_registration_data,
-):
-    """Test registration with all organizations selected"""
-    data = valid_registration_data.copy()
-    data["organizations"] = {k: True for k in mock_settings.organizations.keys()}
-
-    user_data = Auth0UserDataFactory.build()
-    mock_auth0_client.create_user.return_value = user_data
-
-    email_service_cls = mocker.patch("routers.bpa_register.EmailService", autospec=True)
-    email_service_cls.return_value.send.return_value = True
-
-    response = test_client_with_email.post("/bpa/register", json=data)
-
-    assert response.status_code == 200
-    called_data = mock_auth0_client.create_user.call_args[0][0]
-    bpa_service = called_data.app_metadata.services[0]
-    assert len(bpa_service.resources) == len(mock_settings.organizations)
-
-    email_service_cls.return_value.send.assert_called_once()