Feature/site group management and details by ram-from-tvl · Pull Request #247 · openclimatefix/pv-site-datamodel

ram-from-tvl · 2025-07-31T06:00:01Z

Add Site Group Management and Details Modules

Fixes #231
This PR introduces two new modules to enhance the PVSite datamodel with better site group management capabilities and detailed information retrieval functions.

Management Module (`src/pvsite_datamodel/management.py`)

Site selection and validation by UUID or client ID
Bulk operations for retrieving all site UUIDs and client IDs
Site group management functions (add sites, update user groups)
Utility for adding all sites to a specific group
Basic email validation helper

Details Module (`src/pvsite_datamodel/read/details.py`)

Comprehensive user details retrieval (sites, groups, counts)
Detailed site information with all attributes formatted
Site group details with associated sites and users
Email validation utility

Testing & Documentation

Complete test coverage for both modules
Usage examples demonstrating all functionality
Updated package imports to expose new functions

Files Changed

src/pvsite_datamodel/read/site_group.py (new)
src/pvsite_datamodel/read/details.py (new)
src/pvsite_datamodel/__init__.py (updated exports)
src/pvsite_datamodel/read/__init__.py (updated exports)
tests/test_management.py (new)
tests/read/test_details.py (new)
examples/usage_example.py (new)

Would appreciate any feedback or suggestions for improvements!

- Add management.py with functions for site group operations: * Site selection by UUID and client ID with validation * Get all site UUIDs and client site IDs * Update site groups and user site group assignments * Add all sites to a site group with bulk operations * Email validation utility - Add read/details.py with functions for retrieving detailed information: * Get comprehensive user details including sites and groups * Get detailed site information with all attributes * Get site group details with associated sites and users * Email validation utility - Update package __init__.py files to expose new functions - Add comprehensive test coverage for both modules - Add usage example demonstrating all new functionality These modules provide a clean API for managing site groups and retrieving detailed information about users, sites, and site groups in the PVSite database.

- Remove unused imports (Union, datetime, pytest, LocationAssetType) - Fix lines exceeding 100 characters by breaking long lines - Improve exception handling with 'raise ... from err' pattern - Remove examples folder as requested - Clean up whitespace and formatting issues All flake8 violations have been addressed: - T201: Removed print statements by deleting examples - E501: Fixed line length violations - F401: Removed unused imports - W293: Cleaned up blank line whitespace - B904: Fixed exception re-raising pattern

- Fix long line in details.py docstring (E501) - Remove unused imports from test files (F401): * get_site_group_details from test_details.py * update_site_group and change_user_site_group from test_management.py - Clean up trailing whitespace on blank lines (W293) - Ensure proper line endings All flake8 violations should now be resolved.

- Add extra blank line in module docstring for better formatting - Maintain consistency with Python docstring conventions

- Fix D205 docstring style requirement - Add blank line between summary and description in details.py - Ensure proper docstring formatting compliance

ram-from-tvl · 2025-07-31T07:03:14Z

Hi @peterdudfield ,
I believe I've resolved all the lint and CI issues. Could you please take a look at the PR for the issue #231 and let me know if anything else is required? I'll update the documentation as discussed as mentioned in #232 once it's finalized.
Thank you!

peterdudfield · 2025-07-31T10:41:49Z

I think most of the functions should be put in here is they are to do with sites. Please note that some functions are already move over here. Could you onyl add the ones that are not in here arleady

- Move get_all_site_uuids, get_all_client_site_ids, and get_site_details from management.py and details.py to read/site.py - Remove redundant functions from management.py that overlapped with existing site.py functions - Update import structure in __init__.py files to maintain API compatibility - Addresses feedback from PR review to consolidate site-related functions in appropriate module Fixes organizational structure as requested in PR review feedback.

- Addresses linting error F401 for unused import - Cleaned up imports to only include functions that are actually used

ram-from-tvl · 2025-07-31T11:13:28Z

I think most of the functions should be put in here is they are to do with sites. Please note that some functions are already move over here. Could you onyl add the ones that are not in here arleady

I have made some reorganisation and updated the code. please let me know if anything is redundant and requires changes.
Thankyou!

- Added get_site_details function in src/pvsite_datamodel/read/details.py - Function returns comprehensive site information as expected by tests - Fixed import to use get_site_by_uuid from site module instead of non-existent get_site_details - Handles asset type enum conversion and formatting - Includes all site properties like capacity, location, groups, etc.

- Remove trailing whitespace from blank lines - Split long line 95 to comply with 100 character limit - Fix W293 and E501 linting errors

- Fix test_select_site_by_client_id_success to mock the correct function select_site_by_client_id - Add test_select_site_by_client_id_not_found for error case - Fix TestGetAllSites tests to mock the correct functions instead of get_all_sites - Clean up duplicate imports in management.py - Import functions directly instead of using wrapper functions Fixes: - AttributeError: get_site_by_client_site_id does not exist - TypeError: Mock objects not behaving like proper lists in get_all_* functions

- Remove unused imports for get_all_site_uuids, get_all_client_site_ids, get_site_by_client_site_id - Shorten long import line to comply with 100 character limit - Add get_all_site_uuids and get_all_client_site_ids functions using local imports - Keep select_site_by_client_id using direct database query implementation Fixes: - F401 imported but unused errors - E501 line too long error

- Fix test_select_site_by_client_id_success to properly mock database query instead of function - Fix test_select_site_by_client_id_not_found to mock query returning None - Fix test_get_all_site_uuids and test_get_all_client_site_ids to mock underlying functions in read.site module - Remove problematic function-level mocking that was causing Mock objects to be returned instead of expected types Tests now properly mock the database interactions and return expected data types.

peterdudfield · 2025-08-04T11:14:41Z

src/pvsite_datamodel/site_group.py

@@ -0,0 +1,193 @@
+"""This module contains functions for managing site groups."""


could this be moved int the read folder?

peterdudfield · 2025-08-04T11:15:20Z

src/pvsite_datamodel/read/details.py

+from pvsite_datamodel.read.site import get_site_by_uuid
+
+
+def get_user_details(


this can stay in the dashboard

Hi @peterdudfield
Is it better to remove details.py from the read folder.
Thankyou!

Hi @peterdudfield
Could you please hare more information on this PR. I have made some changes so far. please let me know if its good to go..?
Thank you!

peterdudfield · 2025-08-04T11:15:35Z

src/pvsite_datamodel/read/details.py

+    return user_sites, user_site_group, user_site_count
+
+
+def get_site_group_details(


this can stay in the dashboard

peterdudfield · 2025-08-04T11:15:44Z

src/pvsite_datamodel/read/details.py

+    return site_group_sites, site_group_users
+
+
+def validate_email(email: str) -> bool:


should this not be in user.py

oh, youve moved it over there arleady, so remove this

peterdudfield · 2025-08-04T11:15:51Z

src/pvsite_datamodel/read/details.py

+    return False
+
+
+def get_site_details(session, site_uuid: str) -> Dict[str, str]:


keep this in the dashboard

peterdudfield · 2025-08-04T11:17:22Z

tests/test_site_group.py

@@ -0,0 +1,178 @@
+"""Tests for the site_group module."""


move inside read folder

peterdudfield · 2025-08-04T11:17:39Z

tests/test_site_group.py

+    assert user_site_group == "new_site_group"
+
+
+def test_validate_email():


this is good

but you should remove it, as its bee moved to a different rest function already

peterdudfield · 2025-08-04T11:18:36Z

tests/test_site_group.py

+                                         select_site_by_uuid)
+
+
+@patch("pvsite_datamodel.site_group.get_site_by_uuid")


remove all mocking, see how other tests have been done

peterdudfield · 2025-08-04T11:18:54Z

tests/read/test_get_api_requests.py

 import datetime as dt

-from pvsite_datamodel import APIRequestSQL
+from pvsite_datamodel.sqlmodels import APIRequestSQL


undo all these changes from pvsite_datamodel import APIRequestSQL is fine

peterdudfield · 2025-08-04T11:19:01Z

tests/read/test_get_user_by_email.py

        assert len(db_session.query(UserSQL).all()) == 1
+
+
+class TestValidateEmail:


this is good

peterdudfield

Ive added lots of comments. Please dont resolve conversation if they havent been done
Thank you for doing these

ram-from-tvl · 2025-08-07T05:05:54Z

Ive added lots of comments. Please dont resolve conversation if they havent been done Thank you for doing these

Hi @peterdudfield
Can i remove details.py and proceed further.
Thankyou!

ram-from-tvl · 2025-08-08T04:05:29Z

Hi @peterdudfield,
Any updates on this PR..?
Thank you!

…group - Remove get_all_site_uuids from main __init__.py imports and __all__ list - Add get_all_site_uuids function to site_group.py using get_all_sites - This resolves ImportError: cannot import name 'get_all_site_uuids'

- Remove get_site_details from imports in __init__.py - Remove get_site_details from __all__ list - Function doesn't exist in codebase, causing ImportError - Resolves: ImportError: cannot import name 'get_site_details'

- Add validate_email import to src/pvsite_datamodel/read/__init__.py - Function exists in user.py but wasn't exported from read module - Resolves: ImportError: cannot import name 'validate_email' from 'pvsite_datamodel.read' - Fixes test_get_user_by_email.py import error

ram-from-tvl · 2025-08-22T06:42:11Z

Hi @peterdudfield
I have made most of requested changes and have also replied to the requested changes.
And changing this import statement from pvsite_datamodel.sqlmodels import APIRequestSQL creates some circular dependency issue. Can we just keep these statements as it is. Other than these I have made all the other changes.
please let me know is this is good to go. The tests are also passed.
Thank you!

ram-from-tvl added 3 commits July 30, 2025 10:35

url validation bug is fixed

518f694

ram-from-tvl force-pushed the feature/site-group-management-and-details branch from 1c67378 to 0f7f795 Compare July 31, 2025 06:14

ram-from-tvl force-pushed the feature/site-group-management-and-details branch from 5135a25 to d7c3926 Compare July 31, 2025 06:22

docs: Update details.py docstring formatting

fdcedb2

- Add extra blank line in module docstring for better formatting - Maintain consistency with Python docstring conventions

ram-from-tvl force-pushed the feature/site-group-management-and-details branch from 25cd8d3 to fdcedb2 Compare July 31, 2025 06:26

fix: Add required blank line in module docstring

d9f4e73

- Fix D205 docstring style requirement - Add blank line between summary and description in details.py - Ensure proper docstring formatting compliance

ram-from-tvl force-pushed the feature/site-group-management-and-details branch from 81dfbe4 to d9f4e73 Compare July 31, 2025 06:28

Fix circular imports

4436cf9

ram-from-tvl force-pushed the feature/site-group-management-and-details branch from 594a8c6 to 4436cf9 Compare July 31, 2025 06:39

Fix test imports

a015ab7

ram-from-tvl force-pushed the feature/site-group-management-and-details branch from ce25934 to a015ab7 Compare July 31, 2025 06:43

ram-from-tvl added 3 commits July 31, 2025 12:13

Merge branch 'main' into feature/site-group-management-and-details

d26826a

Fix forecast imports

6e82ade

Fix mock setup

def3a61

ram-from-tvl mentioned this pull request Jul 31, 2025

dashboard functions - here #231

Open

ram-from-tvl added 2 commits July 31, 2025 16:38

fix: Remove unused import get_site_by_client_site_id from management.py

8c87ec2

- Addresses linting error F401 for unused import - Cleaned up imports to only include functions that are actually used

ram-from-tvl added 7 commits July 31, 2025 17:01

resolved some CI issues

1f40395

Ci issues again resolved

30c93b6

Fix style errors in details.py

565dc23

- Remove trailing whitespace from blank lines - Split long line 95 to comply with 100 character limit - Fix W293 and E501 linting errors

peterdudfield reviewed Aug 4, 2025

View reviewed changes

tests/test_site_group.py

@@ -0,0 +1,178 @@

"""Tests for the site_group module."""

Copy link

Contributor

peterdudfield Aug 4, 2025

Choose a reason for hiding this comment

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

move inside read folder

peterdudfield reviewed Aug 4, 2025

View reviewed changes

tests/read/test_get_user_by_email.py

assert len(db_session.query(UserSQL).all()) == 1

class TestValidateEmail:

Copy link

Contributor

peterdudfield Aug 4, 2025

Choose a reason for hiding this comment

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

this is good

peterdudfield requested changes Aug 4, 2025

View reviewed changes

ram-from-tvl added 5 commits August 5, 2025 11:33

made some requested changes

be8700b

lint issues

3226329

lint issues

31d16b9

remove old files

8459dca

solved test issues

d6e4a19

ram-from-tvl added 10 commits August 22, 2025 11:28

made some changes

99ed415

solved ci issues

23474f7

solved test issues

5f18c55

solved test issues

b31278e

solved ci test issues

569d395

solved ci test issues

f8d0d17

solved ci test issues

7a21f92

fix: remove non-existent get_site_details import

82d1b56

- Remove get_site_details from imports in __init__.py - Remove get_site_details from __all__ list - Function doesn't exist in codebase, causing ImportError - Resolves: ImportError: cannot import name 'get_site_details'

Merge branch 'main' into feature/site-group-management-and-details

1d7f90f

		@@ -0,0 +1,193 @@
		"""This module contains functions for managing site groups."""

		from pvsite_datamodel.read.site import get_site_by_uuid


		def get_user_details(

		return user_sites, user_site_group, user_site_count


		def get_site_group_details(

		return site_group_sites, site_group_users


		def validate_email(email: str) -> bool:

		return False


		def get_site_details(session, site_uuid: str) -> Dict[str, str]:

		assert user_site_group == "new_site_group"


		def test_validate_email():

		select_site_by_uuid)


		@patch("pvsite_datamodel.site_group.get_site_by_uuid")

		assert len(db_session.query(UserSQL).all()) == 1


		class TestValidateEmail:

Uh oh!

Conversation

ram-from-tvl commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Add Site Group Management and Details Modules

Management Module (src/pvsite_datamodel/management.py)

Details Module (src/pvsite_datamodel/read/details.py)

Testing & Documentation

Files Changed

Uh oh!

ram-from-tvl commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peterdudfield commented Jul 31, 2025

Uh oh!

ram-from-tvl commented Jul 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterdudfield left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ram-from-tvl commented Aug 7, 2025

Uh oh!

ram-from-tvl commented Aug 8, 2025

Uh oh!

ram-from-tvl commented Aug 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ram-from-tvl commented Jul 31, 2025 •

edited

Loading

Management Module (`src/pvsite_datamodel/management.py`)

Details Module (`src/pvsite_datamodel/read/details.py`)

ram-from-tvl commented Jul 31, 2025 •

edited

Loading

peterdudfield left a comment •

edited

Loading