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

Validation for private projects and confidential leads #1504

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

susilnem
Copy link
Member

@susilnem susilnem commented Jun 6, 2024

Addresses:

Changes

  • Add validation for private projects on Topic model and Automatic summary
  • Add filter lead confidential before sending on NLPServer on both topic model and automatic summary

Mention related users here if any.

This PR doesn't introduce any:

  • temporary files, auto-generated files or secret keys
  • n+1 queries
  • flake8 issues
  • print
  • typos
  • unwanted comments

This PR contains valid:

  • tests
  • permission checks (tests here too)
  • translations

@susilnem susilnem force-pushed the feature/restrict-autoclustering-summarization branch from ecd9f54 to 35b3b81 Compare June 12, 2024 06:52
@susilnem susilnem marked this pull request as ready for review June 12, 2024 09:02
apps/analysis/tasks.py Show resolved Hide resolved
apps/analysis/serializers.py Outdated Show resolved Hide resolved
@@ -392,9 +419,11 @@ def test_automatic_summary(self, trigger_results_mock, RequestHelperMock):
lead1 = LeadFactory.create(project=self.project)
lead2 = LeadFactory.create(project=self.project)
another_lead = LeadFactory.create(project=self.another_project)
lead3 = LeadFactory.create(project=self.private_project)
Copy link
Member

Choose a reason for hiding this comment

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

Let's create another confidential leads RESTRICTED, CONFIDENTIAL and some entries. Make sure the generated files doesn't include those entries, even if filters allows.

Similar to what we did before here https://github.com/the-deep/server/pull/1502/files#diff-223594694777faf05fef2c6acee1587a281db5d1002813ca7b0e43862d6feda0R256-R268

Copy link
Member Author

Choose a reason for hiding this comment

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

For this one, Let's merge the Mentioned PR and Update the fields validation. They are on same function that's why.

Copy link
Member

Choose a reason for hiding this comment

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

@AdityaKhatri Will merge this #1502. After that, let's confirm the above changes.

@susilnem susilnem force-pushed the feature/restrict-autoclustering-summarization branch 2 times, most recently from ad9e2f9 to afd6d2b Compare June 12, 2024 09:52
@AdityaKhatri AdityaKhatri force-pushed the feature/restrict-autoclustering-summarization branch from afd6d2b to 0280d25 Compare June 13, 2024 03:56
@susilnem susilnem force-pushed the feature/restrict-autoclustering-summarization branch from 0280d25 to 39d2fec Compare June 14, 2024 11:07
PR changes
Update side_effect function for checking Confidential leads
@susilnem susilnem force-pushed the feature/restrict-autoclustering-summarization branch from 39d2fec to 388a807 Compare June 14, 2024 11:10
@susilnem susilnem requested a review from k9845 June 18, 2024 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants