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

3399 controlled access fra data files page #3430

Open
wants to merge 28 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5ffa00c
added new grouping to model enum
raftmsohani Jan 7, 2025
c9d2480
added tests
raftmsohani Jan 8, 2025
a8dd63d
added chack for FRA file
raftmsohani Jan 8, 2025
b9f8331
Added API test
raftmsohani Jan 8, 2025
2027bad
Update serializers.py
raftmsohani Jan 8, 2025
0cc815c
Merge branch 'develop' into 3397-FRA-report-types-dataFile-model
raftmsohani Jan 8, 2025
9f93409
skip parsing
raftmsohani Jan 8, 2025
e0859a7
fix linting
raftmsohani Jan 9, 2025
486b19a
Merge branch '3397-FRA-report-types-dataFile-model' into 3399-control…
raftmsohani Jan 15, 2025
6f18151
added fra feature flag and filtering
raftmsohani Jan 15, 2025
75405a9
added functional test
raftmsohani Jan 16, 2025
cd1a074
task file
raftmsohani Jan 16, 2025
e1a9651
task file
raftmsohani Jan 16, 2025
8e75ee7
task file
raftmsohani Jan 16, 2025
05c20ce
Merge branch 'develop' into 3399-controlled-access-FRA-Data-Files-page-3
raftmsohani Jan 17, 2025
fc8e14a
fixed failing e2e
raftmsohani Jan 17, 2025
54d3b7e
increased timeout
raftmsohani Jan 17, 2025
dc3cd9d
fix problems with e2e
raftmsohani Jan 21, 2025
4ad6999
Merge branch 'develop' into 3399-controlled-access-FRA-Data-Files-page-3
raftmsohani Jan 21, 2025
093c734
Update tdrs-backend/tdpservice/users/migrations/0042_user_feature_fla…
raftmsohani Jan 21, 2025
0356b72
improved test and some linting
raftmsohani Jan 27, 2025
b353c2f
correct null field
raftmsohani Jan 28, 2025
2bd0f18
Merge branch 'develop' into 3399-controlled-access-FRA-Data-Files-page-3
raftmsohani Jan 28, 2025
a5795c5
Merge branch 'develop' into 3399-controlled-access-FRA-Data-Files-page-3
raftmsohani Jan 29, 2025
1e672fa
Added FRA filter for user with FRA access
raftmsohani Jan 29, 2025
2b0e4be
changed filtering
raftmsohani Jan 29, 2025
8edc8e0
Merge branch 'develop' into 3399-controlled-access-FRA-Data-Files-page-3
raftmsohani Feb 7, 2025
9c47936
Merge branch 'develop' into 3399-controlled-access-FRA-Data-Files-page-3
ADPennington Feb 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ tasks:
desc: Run cypress tests
dir: tdrs-frontend
cmds:
- docker-compose -f docker-compose.local.yml up --build tdp-frontend-test -d
- docker compose -f docker-compose.local.yml up --build tdp-frontend-test -d
- npm run test:e2e

k6:
Expand Down
15 changes: 15 additions & 0 deletions tdrs-backend/tdpservice/data_files/admin/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,21 @@ class Media:

actions = ['reparse']

def get_queryset(self, request):
Copy link
Author

@raftmsohani raftmsohani Jan 29, 2025

Choose a reason for hiding this comment

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

if user doesn't have FRA access or is Admin, then FRA files will be filtered

"""Return the queryset."""
qs = super().get_queryset(request)
# return data files based on user's section
FRA_SECTION_LIST = [
DataFile.Section.FRA_WORK_OUTCOME_TANF_EXITERS,
DataFile.Section.FRA_SECONDRY_SCHOOL_ATTAINMENT,
DataFile.Section.FRA_SUPPLEMENT_WORK_OUTCOMES
]
if not request.user.has_fra_access:
filtered_for_fra = qs.exclude(section__in=FRA_SECTION_LIST)
return filtered_for_fra
else:
return qs.filter(section__in=FRA_SECTION_LIST)
Copy link

Choose a reason for hiding this comment

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

not sure if this is intended, but setting fra_access=True means that the data files page filters down to only FRA files. you have to set fra_access=False to see other file types. maybe we should have a "both" option for admins and other users so that they don't have to switch the feature flag to find files in the admin


def reparse(self, request, queryset):
"""Reparse the selected data files."""
files = queryset.values_list("id", flat=True)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 3.2.15 on 2025-01-08 13:42

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('data_files', '0016_remove_datafile_reparse_meta_models'),
]

operations = [
migrations.AlterField(
model_name='datafile',
name='section',
field=models.CharField(choices=[('Tribal Closed Case Data', 'Tribal Closed Case Data'), ('Tribal Active Case Data', 'Tribal Active Case Data'), ('Tribal Aggregate Data', 'Tribal Aggregate Data'), ('Tribal Stratum Data', 'Tribal Stratum Data'), ('SSP Aggregate Data', 'Ssp Aggregate Data'), ('SSP Closed Case Data', 'Ssp Closed Case Data'), ('SSP Active Case Data', 'Ssp Active Case Data'), ('SSP Stratum Data', 'Ssp Stratum Data'), ('Active Case Data', 'Active Case Data'), ('Closed Case Data', 'Closed Case Data'), ('Aggregate Data', 'Aggregate Data'), ('Stratum Data', 'Stratum Data'), ('Work Outcomes for TANF Exiters', 'Fra Work Outcome Tanf Exiters'), ('Secondary School Attainment', 'Fra Secondry School Attainment'), ('Supplemental Work Outcomes', 'Fra Supplement Work Outcomes')], max_length=32),
),
]
50 changes: 46 additions & 4 deletions tdrs-backend/tdpservice/data_files/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,49 @@ class Section(models.TextChoices):
AGGREGATE_DATA = "Aggregate Data"
STRATUM_DATA = "Stratum Data"

FRA_WORK_OUTCOME_TANF_EXITERS = "Work Outcomes for TANF Exiters"
FRA_SECONDRY_SCHOOL_ATTAINMENT = "Secondary School Attainment"
FRA_SUPPLEMENT_WORK_OUTCOMES = "Supplemental Work Outcomes"

@classmethod
def is_fra(cls, section: str) -> bool:
"""Determine if the section is a FRA section."""
return section in [
cls.FRA_WORK_OUTCOME_TANF_EXITERS,
cls.FRA_SECONDRY_SCHOOL_ATTAINMENT,
cls.FRA_SUPPLEMENT_WORK_OUTCOMES
]

@classmethod
def is_ssp(cls, section: str) -> bool:
"""Determine if the section is an SSP section."""
return section in [
cls.SSP_AGGREGATE_DATA,
cls.SSP_ACTIVE_CASE_DATA,
cls.SSP_CLOSED_CASE_DATA,
cls.SSP_STRATUM_DATA
]

@classmethod
def is_tribal(cls, section: str) -> bool:
"""Determine if the section is a Tribal section."""
return section in [
cls.TRIBAL_AGGREGATE_DATA,
cls.TRIBAL_ACTIVE_CASE_DATA,
cls.TRIBAL_CLOSED_CASE_DATA,
cls.TRIBAL_STRATUM_DATA
]

@classmethod
def is_tanf(cls, section: str) -> bool:
"""Determine if the section is a TANF section."""
return section in [
cls.ACTIVE_CASE_DATA,
cls.CLOSED_CASE_DATA,
cls.AGGREGATE_DATA,
cls.STRATUM_DATA
]

class Quarter(models.TextChoices):
"""Enum for data file Quarter."""

Expand Down Expand Up @@ -182,14 +225,13 @@ class Meta:
def prog_type(self):
"""Return the program type for a given section."""
# e.g., 'SSP Closed Case Data'
if self.section.startswith('SSP'):
if self.Section.is_ssp(self.section):
return 'SSP'
elif self.Section.is_fra(self.section):
return 'FRA'
else:
return 'TAN'

# TODO: if given a datafile (section), we can reverse back to the program b/c the
# section string has "tribal/ssp" in it, then process of elimination we have tanf

@property
def filename(self):
"""Return the correct filename for this data file."""
Expand Down
6 changes: 6 additions & 0 deletions tdrs-backend/tdpservice/data_files/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,9 @@ def validate_file(self, file):
validate_file_extension(file.name)
validate_file_infection(file, file.name, user)
return file

def validate_section(self, section):
"""Validate the section field."""
if DataFile.Section.is_fra(section):
raise serializers.ValidationError("Section cannot be FRA")
return section
21 changes: 21 additions & 0 deletions tdrs-backend/tdpservice/data_files/test/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ def assert_data_file_created(response):
"""Assert that the data file was created."""
assert response.status_code == status.HTTP_201_CREATED

@staticmethod
def assert_data_file_error(response):
"""Assert that the data file was created."""
assert response.status_code == status.HTTP_400_BAD_REQUEST

@staticmethod
def assert_data_file_rejected(response):
"""Assert that a given data file submission was rejected."""
Expand Down Expand Up @@ -154,6 +159,15 @@ def post_data_file_file(self, api_client, data_file_data):
format='multipart'
)

def post_data_file_fra(self, api_client, data_file_data):
"""Submit a data file with the given data."""
data_file_data['section'] = 'Secondary School Attainment'
return api_client.post(
self.root_url,
data_file_data,
format='multipart'
)

def get_data_file_files(self, api_client):
"""Submit a data file with the given data."""
return api_client.get(
Expand Down Expand Up @@ -213,6 +227,13 @@ def test_create_data_file_file_entry(self, api_client, data_file_data, user):
self.assert_data_file_created(response)
self.assert_data_file_exists(data_file_data, 1, user)

def test_create_data_file_fra(self, api_client, data_file_data, user):
"""Test ability to create data file metadata registry."""
response = self.post_data_file_fra(api_client, data_file_data)
from rest_framework.exceptions import ErrorDetail
Copy link

Choose a reason for hiding this comment

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

Is there a reason this lives in the function instead of at the module level?

Copy link
Author

Choose a reason for hiding this comment

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

not really, corrected it

assert response.data == {'section': [ErrorDetail(string='Section cannot be FRA', code='invalid')]}
self.assert_data_file_error(response)

def test_data_file_file_version_increment(
self,
api_client,
Expand Down
20 changes: 14 additions & 6 deletions tdrs-backend/tdpservice/data_files/test/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,18 @@ def test_data_files_filename_is_expected(user):
assert new_data_file.filename == stt.filenames[section]

@pytest.mark.django_db
def test_prog_type(data_file_instance):
@pytest.mark.parametrize('section, program_type',
[('Tribal Closed Case Data', 'TAN'),
('Tribal Active Case Data', 'TAN'),
('SSP Aggregate Data', 'SSP'),
('SSP Closed Case Data', 'SSP'),
('Active Case Data', 'TAN'),
('Aggregate Data', 'TAN'),
('Work Outcomes for TANF Exiters', 'FRA'),
('Secondary School Attainment', 'FRA'),
('Supplemental Work Outcomes', 'FRA')
])
def test_prog_type(data_file_instance, section, program_type):
"""Test propert prog_type."""
df = DataFile.create_new_version({
"year": data_file_instance.year,
Expand All @@ -97,11 +108,8 @@ def test_prog_type(data_file_instance):
"user": data_file_instance.user,
})

assert df.prog_type == "TAN"
df.section = "SSP Active Case Data"
assert df.prog_type == "SSP"
df.section = "Tribal Active Case Data"
assert df.prog_type == "TAN"
df.section = section
assert df.prog_type == program_type

@pytest.mark.django_db
def test_fiscal_year(data_file_instance):
Expand Down
12 changes: 12 additions & 0 deletions tdrs-backend/tdpservice/data_files/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class DataFileViewSet(ModelViewSet):
def create(self, request, *args, **kwargs):
"""Override create to upload in case of successful scan."""
logger.debug(f"{self.__class__.__name__}: {request}")

response = super().create(request, *args, **kwargs)

# only if file is passed the virus scan and created successfully will we perform side-effects:
Expand All @@ -69,6 +70,10 @@ def create(self, request, *args, **kwargs):
f"Datafile META -> datafile: {data_file_id}, section: {data_file.section}, " +
f"quarter {data_file.quarter}, year {data_file.year}.")

if data_file.prog_type == 'FRA':
logger.debug(f"{self.__class__.__name__}: return val: {response}")
return response

parser_task.parse.delay(data_file_id)
logger.info("Submitted parse task to queue for datafile %s.", data_file_id)

Expand Down Expand Up @@ -97,9 +102,16 @@ def get_s3_versioning_id(self, file_name, prefix):
def get_queryset(self):
"""Apply custom queryset filters."""
queryset = super().get_queryset().order_by('-created_at')
FRA_SECTION_LIST = [
DataFile.Section.FRA_WORK_OUTCOME_TANF_EXITERS,
DataFile.Section.FRA_SECONDRY_SCHOOL_ATTAINMENT,
DataFile.Section.FRA_SUPPLEMENT_WORK_OUTCOMES
]
if self.action == 'list':
if self.request.query_params.get('file_type') == 'ssp-moe':
queryset = queryset.filter(section__contains='SSP')
elif self.request.query_params.get('file_type') == 'fra':
queryset = queryset.filter(section__in=FRA_SECTION_LIST)
else:
queryset = queryset.exclude(section__contains='SSP')

Expand Down
5 changes: 5 additions & 0 deletions tdrs-backend/tdpservice/users/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

from .models import User

import logging
logger = logging.getLogger()

class UserForm(forms.ModelForm):
"""Customize the user admin form."""
Expand All @@ -19,6 +21,9 @@ class Meta:
exclude = ['password', 'user_permissions']
readonly_fields = ['last_login', 'date_joined', 'login_gov_uuid', 'hhs_id', 'access_request']

def __init__(self, *args, **kwargs):
Copy link

Choose a reason for hiding this comment

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

Is this stylistic/for completeness, or was this required?

Copy link
Author

Choose a reason for hiding this comment

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

byproduct of an approach I was taking, can be removed

super().__init__(*args, **kwargs)

def clean(self):
"""Add extra validation for locations based on roles."""
cleaned_data = super().clean()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 3.2.15 on 2025-01-21 04:29

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('users', '0041_users_digit_group_add_datafile_permission'),
]

operations = [
migrations.AddField(
model_name='user',
name='feature_flags',
field=models.JSONField(blank=True, default=dict, help_text='Feature flags for this data file. JSON formatting example: {"fra_access": "True"}', null=True),
),
]
13 changes: 13 additions & 0 deletions tdrs-backend/tdpservice/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,19 @@ class Meta:
_loaded_values = None
_adding = True

# Feature flag for the user to enable or disable FRA access
feature_flags = models.JSONField(
default=dict,
help_text="Feature flags for this data file.",
null=True,
blank=True,
Copy link

Choose a reason for hiding this comment

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

If we keep null and blank set to True, the property has_fra_access should do a null check on self.feature_flags before checking for the key. I was able to leave the form field blank in the admin and cause an exception to be raised when navigating to the data files page. Could we instead make them both false since we do have the default set as dict to avoid worrying about checks?

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately, there is an internal check for {} in JSONField. The only way to bypass is to override JSONField, but I decided to check for null or empty dict during cleaning which is simple

Copy link
Author

Choose a reason for hiding this comment

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

Also moved null out

)

@property
def has_fra_access(self):
"""Return whether or not the user has FRA access."""
return self.feature_flags.get('fra_access', False)

def __str__(self):
"""Return the username as the string representation of the object."""
return self.username
Expand Down
33 changes: 33 additions & 0 deletions tdrs-backend/tdpservice/users/test/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
import pytest

from tdpservice.stts.models import STT, Region
from tdpservice.data_files.models import DataFile
from tdpservice.data_files.test.factories import DataFileFactory
from tdpservice.users.models import User
from django.test import Client


@pytest.mark.django_db
Expand Down Expand Up @@ -70,3 +74,32 @@ def test_user_can_only_have_stt_or_region(user, stt, region):

user.clean()
user.save()

@pytest.mark.django_db
def test_user_with_fra_access(client, user, stt):
"""Test that a user with FRA access can only have an STT."""
user.stt = stt
user.is_superuser = True
user.feature_flags = {"fra_access": False}

user.clean()
user.save()
raftmsohani marked this conversation as resolved.
Show resolved Hide resolved

user = User.objects.create_superuser('admin', '[email protected]', 'password')
client = Client()
client.force_login(user)

# Need a datafile, with a section that is not in the FRA_SECTION_LIST
Copy link

Choose a reason for hiding this comment

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

This comment is confusing since the test is using an FRA section immediately below it even though the comment says not to.

datafile = DataFileFactory()
datafile.section = DataFile.Section.FRA_WORK_OUTCOME_TANF_EXITERS
datafile.save()

response = client.get(f"/admin/data_files/datafile/{datafile.id}/change/")
assert response.status_code != 200
assert f'Data file with ID “{datafile.id}” doesn’t exist. Perhaps it was deleted?'

user.feature_flags = {"fra_access": True}
user.save()

response = client.get(f"/admin/data_files/datafile/{datafile.id}/change/")
assert response.status_code == 200
2 changes: 1 addition & 1 deletion tdrs-frontend/cypress/e2e/accounts/accounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ When('{string} requests access', (username) => {
})

cy.get('button').contains('Request Access').should('exist').click()
cy.wait(2000).then(() => {
cy.wait(4000).then(() => {
cy.contains('Request Submitted').should('exist')
})
})
Expand Down
Loading