langgenius/dify
1
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-07-21 12:05:30 -04:00
Code Issues 775 Packages Projects Releases 10 Wiki Activity

Add unit tests for dataset permission service #20599

New Issue
Closed
opened 2026-02-21 20:08:06 -05:00 by yindo · 1 comment
yindo commented 2026-02-21 20:08:06 -05:00
Owner
Copy Link
Copy Source

Originally created by @SmartDever02 on GitHub (Nov 26, 2025).

Self Checks

  • I have read the Contributing Guide and Language Policy.
  • I have searched for existing issues search for existing issues, including closed ones.
  • I confirm that I am using English to submit this report, otherwise it will be closed.
  • Please do not modify this template :) and fill in all the required fields.

1. Is this request related to a challenge you're experiencing? Tell me about your story.

We need comprehensive unit test coverage for the Dataset permission service methods that are not yet covered by existing test suites.

Currently, we have test coverage for dataset creation, retrieval, update/delete operations, document management, segment management, hit testing, external datasets, collection bindings, metadata, and controller APIs. However, the dataset permission management functionality remains untested, including partial member list operations, permission validation logic, and permission enum handling.

This makes it difficult to safely refactor or extend permission-related functionality (e.g., permission validation changes, partial member list updates, permission enum modifications) without risking regressions that could affect data security and access control.

The Dataset permission service provides critical functionality for:

  • Managing partial member permissions for datasets
  • Validating user permissions before dataset operations
  • Handling different permission levels (only_me, all_team_members, partial_members)
  • Clearing partial member lists when needed

This issue proposes adding a comprehensive test suite for the following methods:

1. get_dataset_partial_member_list

  • Retrieve partial member permissions for a dataset
  • Return list of users with explicit permissions
  • Handle datasets with no partial members
  • Validate dataset existence

2. update_partial_member_list

  • Add new partial members to dataset
  • Update existing partial member permissions
  • Remove members from partial list
  • Validate member permissions
  • Handle permission enum changes

3. check_permission

  • Validate user has access to dataset
  • Check tenant-level permissions
  • Verify only_me permission restrictions
  • Validate partial_members permission checks
  • Handle all_team_members permission
  • Test permission denial scenarios

4. clear_partial_member_list

  • Clear all partial members from dataset
  • Handle datasets with no partial members
  • Validate permission before clearing
  • Ensure proper cleanup

5. Permission enum handling

  • Test only_me permission behavior
  • Test all_team_members permission behavior
  • Test partial_members permission behavior
  • Validate enum transitions
  • Test edge cases for each permission type

2. Additional context or comments

The test suite should follow the existing patterns used for other service test files, including:

  • Comprehensive test coverage for all methods
  • Proper mocking of dependencies (database, user context, permission models)
  • Error condition testing
  • Edge case handling
  • Detailed documentation and comments
  • Test data factories for reusable mock objects

The tests should be organized into a single file:

  • dataset_permission_service.py - Branch: feat/test-dataset-permission-service

Key testing scenarios to cover:

  • Permission validation for different user roles (OWNER, ADMIN, NORMAL)
  • Partial member list CRUD operations
  • Permission enum transitions and validations
  • Tenant isolation and security
  • Error handling for invalid operations
  • Edge cases (empty lists, missing users, etc.)

3. Can you help us with this feature?

  • I am interested in contributing to this feature.
Originally created by @SmartDever02 on GitHub (Nov 26, 2025). ### Self Checks - [x] I have read the [Contributing Guide](https://github.com/langgenius/dify/blob/main/CONTRIBUTING.md) and [Language Policy](https://github.com/langgenius/dify/issues/1542). - [x] I have searched for existing issues [search for existing issues](https://github.com/langgenius/dify/issues), including closed ones. - [x] I confirm that I am using English to submit this report, otherwise it will be closed. - [x] Please do not modify this template :) and fill in all the required fields. ### 1. Is this request related to a challenge you're experiencing? Tell me about your story. We need comprehensive unit test coverage for the Dataset permission service methods that are not yet covered by existing test suites. Currently, we have test coverage for dataset creation, retrieval, update/delete operations, document management, segment management, hit testing, external datasets, collection bindings, metadata, and controller APIs. However, the dataset permission management functionality remains untested, including partial member list operations, permission validation logic, and permission enum handling. This makes it difficult to safely refactor or extend permission-related functionality (e.g., permission validation changes, partial member list updates, permission enum modifications) without risking regressions that could affect data security and access control. The Dataset permission service provides critical functionality for: - Managing partial member permissions for datasets - Validating user permissions before dataset operations - Handling different permission levels (only_me, all_team_members, partial_members) - Clearing partial member lists when needed This issue proposes adding a comprehensive test suite for the following methods: **1. get_dataset_partial_member_list** - Retrieve partial member permissions for a dataset - Return list of users with explicit permissions - Handle datasets with no partial members - Validate dataset existence **2. update_partial_member_list** - Add new partial members to dataset - Update existing partial member permissions - Remove members from partial list - Validate member permissions - Handle permission enum changes **3. check_permission** - Validate user has access to dataset - Check tenant-level permissions - Verify only_me permission restrictions - Validate partial_members permission checks - Handle all_team_members permission - Test permission denial scenarios **4. clear_partial_member_list** - Clear all partial members from dataset - Handle datasets with no partial members - Validate permission before clearing - Ensure proper cleanup **5. Permission enum handling** - Test only_me permission behavior - Test all_team_members permission behavior - Test partial_members permission behavior - Validate enum transitions - Test edge cases for each permission type ### 2. Additional context or comments The test suite should follow the existing patterns used for other service test files, including: - Comprehensive test coverage for all methods - Proper mocking of dependencies (database, user context, permission models) - Error condition testing - Edge case handling - Detailed documentation and comments - Test data factories for reusable mock objects The tests should be organized into a single file: - `dataset_permission_service.py` - Branch: `feat/test-dataset-permission-service` Key testing scenarios to cover: - Permission validation for different user roles (OWNER, ADMIN, NORMAL) - Partial member list CRUD operations - Permission enum transitions and validations - Tenant isolation and security - Error handling for invalid operations - Edge cases (empty lists, missing users, etc.) ### 3. Can you help us with this feature? - [x] I am interested in contributing to this feature.
yindo added the 💪 enhancement label 2026-02-21 20:08:06 -05:00
yindo closed this issue 2026-02-21 20:08:06 -05:00
yindo commented 2026-02-21 20:08:06 -05:00
Author
Owner
Copy Link
Copy Source

@dosubot[bot] commented on GitHub (Nov 26, 2025):

Hi @SmartDever02! I'm Dosu and I’m helping the dify team.

You’re on the right track—Dify already has a solid foundation for unit testing the dataset permission service. The core logic for permission management (including get_dataset_partial_member_list, update_partial_member_list, check_permission, and clear_partial_member_list) is implemented in api/services/dataset_service.py, and permission enums are defined in api/models/dataset.py.

Tests use pytest and unittest.mock for mocking dependencies, with reusable test data factories like DatasetPermissionTestDataFactory and DatasetServiceTestDataFactory to create mock datasets, users, and permissions. See api/tests/unit_tests/services/test_dataset_permission.py for examples of permission validation, CRUD, enum transitions, error handling, and edge cases.

To add or extend tests:

  • Follow the Arrange-Act-Assert pattern and use fixtures to patch database sessions and logging.
  • Use the test data factories for consistent mock setup.
  • Cover all permission levels (ONLY_ME, ALL_TEAM, PARTIAL_TEAM), CRUD for partial member lists, enum transitions, tenant isolation, and error scenarios.
  • Document each test clearly, following the style in the existing test files.

If you need to test scenarios not yet covered (like more granular CRUD on partial member lists or permission enum edge cases), you can extend the current test suite in the same style. This will ensure robust coverage and safe refactoring for permission logic.

To reply, just mention @dosu.

How did I do? Good | Irrelevant | Incorrect | Verbose | Hallucination | Report 🐛 | Other

Chat with Dosu Open in Cursor Join Discord Share on X

@dosubot[bot] commented on GitHub (Nov 26, 2025): <!-- Greeting --> Hi @SmartDever02! I'm [Dosu](https://go.dosu.dev/dosubot) and I’m helping the dify team. <!-- Answer --> You’re on the right track—Dify already has a solid foundation for unit testing the dataset permission service. The core logic for permission management (including `get_dataset_partial_member_list`, `update_partial_member_list`, `check_permission`, and `clear_partial_member_list`) is implemented in [`api/services/dataset_service.py`](https://github.com/langgenius/dify/blob/8d8800e632a417d21ebaa06e784e66022596a4fc/api/services/dataset_service.py#L3424-L3478), and permission enums are defined in [`api/models/dataset.py`](https://github.com/langgenius/dify/blob/8d8800e632a417d21ebaa06e784e66022596a4fc/api/models/dataset.py#L36-L1143). Tests use `pytest` and `unittest.mock` for mocking dependencies, with reusable test data factories like `DatasetPermissionTestDataFactory` and `DatasetServiceTestDataFactory` to create mock datasets, users, and permissions. See [`api/tests/unit_tests/services/test_dataset_permission.py`](https://github.com/langgenius/dify/blob/8d8800e632a417d21ebaa06e784e66022596a4fc/api/tests/unit_tests/services/test_dataset_permission.py#L1-L306) for examples of permission validation, CRUD, enum transitions, error handling, and edge cases. To add or extend tests: - Follow the Arrange-Act-Assert pattern and use fixtures to patch database sessions and logging. - Use the test data factories for consistent mock setup. - Cover all permission levels (`ONLY_ME`, `ALL_TEAM`, `PARTIAL_TEAM`), CRUD for partial member lists, enum transitions, tenant isolation, and error scenarios. - Document each test clearly, following the style in the existing test files. If you need to test scenarios not yet covered (like more granular CRUD on partial member lists or permission enum edge cases), you can extend the current test suite in the same style. This will ensure robust coverage and safe refactoring for permission logic. <!-- Dosu Comment Footer --> *To reply, just mention [@dosu](https://go.dosu.dev/dosubot).* --- <sup>How did I do? [Good](https://app.dosu.dev/response-feedback/28c53ae5-7329-41c0-8017-dd60e9c52b92?feedback_type=great_response) | [Irrelevant](https://app.dosu.dev/response-feedback/28c53ae5-7329-41c0-8017-dd60e9c52b92?feedback_type=irrelevant_answer) | [Incorrect](https://app.dosu.dev/response-feedback/28c53ae5-7329-41c0-8017-dd60e9c52b92?feedback_type=incorrect_sources) | [Verbose](https://app.dosu.dev/response-feedback/28c53ae5-7329-41c0-8017-dd60e9c52b92?feedback_type=too_verbose) | [Hallucination](https://app.dosu.dev/response-feedback/28c53ae5-7329-41c0-8017-dd60e9c52b92?feedback_type=hallucination) | [Report 🐛](https://app.dosu.dev/response-feedback/28c53ae5-7329-41c0-8017-dd60e9c52b92?feedback_type=bug_report) | [Other](https://app.dosu.dev/response-feedback/28c53ae5-7329-41c0-8017-dd60e9c52b92?feedback_type=other)</sup> [![Chat with Dosu](https://dosu.dev/dosu-chat-badge.svg)](https://app.dosu.dev/6088a4f4-073a-43f3-b3d4-4f06fb0c05a0/ask?utm_source=github)&nbsp;[![Open in Cursor](https://dosu.dev/dosu-in-cursor.svg)](https://cursor.com/link/prompt?text=You%E2%80%99re%20on%20the%20right%20track%E2%80%94Dify%20already%20has%20a%20solid%20foundation%20for%20unit%20testing%20the%20dataset%20permission%20service.%20The%20core%20logic%20for%20permission%20management%20%28including%20%60get_dataset_partial_member_list%60%2C%20%60update_partial_member_list%60%2C%20%60check_permission%60%2C%20and%20%60clear_partial_member_list%60%29%20is%20implemented%20in%20%5B%60api/services/dataset_service.py%60%5D%28https%3A//github.com/langgenius/dify/blob/8d8800e632a417d21ebaa06e784e66022596a4fc/api/services/dataset_service.py%23L3424-L3478%29%2C%20and%20permission%20enums%20are%20defined%20in%20%5B%60api/models/dataset.py%60%5D%28https%3A//github.com/langgenius/dify/blob/8d8800e632a417d21ebaa06e784e66022596a4fc/api/models/dataset.py%23L36-L1143%29.%0A%0ATests%20use%20%60pytest%60%20and%20%60unittest.mock%60%20for%20mocking%20dependencies%2C%20with%20reusable%20test%20data%20factories%20like%20%60DatasetPermissionTestDataFactory%60%20and%20%60DatasetServiceTestDataFactory%60%20to%20create%20mock%20datasets%2C%20users%2C%20and%20permissions.%20See%20%5B%60api/tests/unit_tests/services/test_dataset_permission.py%60%5D%28https%3A//github.com/langgenius/dify/blob/8d8800e632a417d21ebaa06e784e66022596a4fc/api/tests/unit_tests/services/test_dataset_permission.py%23L1-L306%29%20for%20examples%20of%20permission%20validation%2C%20CRUD%2C%20enum%20transitions%2C%20error%20handling%2C%20and%20edge%20cases.%0A%0ATo%20add%20or%20extend%20tests%3A%0A-%20Follow%20the%20Arrange-Act-Assert%20pattern%20and%20use%20fixtures%20to%20patch%20database%20sessions%20and%20logging.%0A-%20Use%20the%20test%20data%20factories%20for%20consistent%20mock%20setup.%0A-%20Cover%20all%20permission%20levels%20%28%60ONLY_ME%60%2C%20%60ALL_TEAM%60%2C%20%60PARTIAL_TEAM%60%29%2C%20CRUD%20for%20partial%20member%20lists%2C%20enum%20transitions%2C%20tenant%20isolation%2C%20and%20error%20scenarios.%0A-%20Document%20each%20test%20clearly%2C%20following%20the%20style%20in%20the%20existing%20test%20files.%0A%0AIf%20you%20need%20to%20test%20scenarios%20not%20yet%20covered%20%28like%20more%20granular%20CRUD%20on%20partial%20member%20lists%20or%20permission%20enum%20edge%20cases%29%2C%20you%20can%20extend%20the%20current%20test%20suite%20in%20the%20same%20style.%20This%20will%20ensure%20robust%20coverage%20and%20safe%20refactoring%20for%20permission%20logic.)&nbsp;[![Join Discord](https://img.shields.io/badge/join-5865F2?logo=discord&logoColor=white&label=)](https://go.dosu.dev/discord-bot)&nbsp;[![Share on X](https://img.shields.io/badge/X-share-black)](https://twitter.com/intent/tweet?text=%40dosu_ai%20helped%20me%20solve%20this%20issue!&url=https%3A//github.com/langgenius/dify/issues/28758)
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Knowledge Pipeline
Proposal
WIP
cloud
dependencies
feat:webapp
good first issue
outdated
performance
pull-request
Mirrored from GitHub Pull Request
 python:uv
refactor
size:L
stale
status: accepting prs
status: blocked
status: in discussion
stt & tts
vulnerablity
web
wont-fix
🌊 feat:workflow
🌚 invalid
🐞 bug
👻 feat:rag
💪 enhancement
📚 documentation
🔨 feat:tools
🙋‍♂️ question
🙏 help wanted
🤔 cant-reproduce
🤖 feat:agent
No Label 💪 enhancement
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: langgenius/dify#20599
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?