From adc2013c7ca1905084dad4fc85d2f933179abcae Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 17 Jun 2024 14:07:12 -0600 Subject: [PATCH 01/21] Simplify report and handle many items --- .../commands/email_current_metadata_report.py | 109 ++++++++++-------- 1 file changed, 62 insertions(+), 47 deletions(-) diff --git a/src/registrar/management/commands/email_current_metadata_report.py b/src/registrar/management/commands/email_current_metadata_report.py index dcaf47b06..1e8faac82 100644 --- a/src/registrar/management/commands/email_current_metadata_report.py +++ b/src/registrar/management/commands/email_current_metadata_report.py @@ -9,7 +9,7 @@ from datetime import datetime from django.core.management import BaseCommand from django.conf import settings from registrar.utility import csv_export -from registrar.utility.s3_bucket import S3ClientHelper +from io import StringIO from ...utility.email import send_templated_email @@ -17,89 +17,104 @@ logger = logging.getLogger(__name__) class Command(BaseCommand): + """Emails a encrypted zip file containing a csv of our domains and domain requests""" help = ( "Generates and uploads a domain-metadata.csv file to our S3 bucket " "which is based off of all existing Domains." ) + current_date = datetime.now().strftime("%m%d%Y") + email_to: str def add_arguments(self, parser): """Add our two filename arguments.""" - parser.add_argument("--directory", default="migrationdata", help="Desired directory") parser.add_argument( - "--checkpath", - default=True, - help="Flag that determines if we do a check for os.path.exists. Used for test cases", + "--emailTo", + default=settings.DEFAULT_FROM_EMAIL, + help="Defines where we should email this report", ) def handle(self, **options): """Grabs the directory then creates domain-metadata.csv in that directory""" - file_name = "domain-metadata.csv" - # Ensures a slash is added - directory = os.path.join(options.get("directory"), "") - check_path = options.get("checkpath") + self.email_to = options.get("emailTo") + + # Don't email to DEFAULT_FROM_EMAIL when not prod. + if not settings.IS_PRODUCTION and self.email_to == settings.DEFAULT_FROM_EMAIL: + raise ValueError( + "The --emailTo arg must be specified in non-prod environments, " + "and the arg must not equal the DEFAULT_FROM_EMAIL value (aka: help@get.gov)." + ) logger.info("Generating report...") + zip_filename = f"domain-metadata-{self.current_date}.zip" try: - self.email_current_metadata_report(directory, file_name, check_path) + self.email_current_metadata_report(zip_filename) except Exception as err: # TODO - #1317: Notify operations when auto report generation fails raise err else: - logger.info(f"Success! Created {file_name} and successfully sent out an email!") + logger.info(f"Success! Created {zip_filename} and successfully sent out an email!") - def email_current_metadata_report(self, directory, file_name, check_path): + def email_current_metadata_report(self, zip_filename): """Creates a current-metadata.csv file under the specified directory, then uploads it to a AWS S3 bucket. This is done for resiliency reasons in the event our application goes down and/or the email cannot send -- we'll still be able to grab info from the S3 instance""" - s3_client = S3ClientHelper() - file_path = os.path.join(directory, file_name) + reports = { + "Domain report": { + "report_filename": f"domain-metadata-{self.current_date}.csv", + "report_function": csv_export.export_data_type_to_csv, + }, + "Domain request report": { + "report_filename": f"domain-request-metadata-{self.current_date}.csv", + "report_function": csv_export.DomainRequestExport.export_full_domain_request_report, + }, + } + # Set the password equal to our content in SECRET_ENCRYPT_METADATA. + # For local development, this will be "devpwd" unless otherwise set. + override = settings.SECRET_ENCRYPT_METADATA is None and not settings.IS_PRODUCTION + password = "devpwd" if override else settings.SECRET_ENCRYPT_METADATA - # Generate a file locally for upload - with open(file_path, "w") as file: - csv_export.export_data_type_to_csv(file) - - if check_path and not os.path.exists(file_path): - raise FileNotFoundError(f"Could not find newly created file at '{file_path}'") - - s3_client.upload_file(file_path, file_name) - - # Set zip file name - current_date = datetime.now().strftime("%m%d%Y") - current_filename = f"domain-metadata-{current_date}.zip" - - # Pre-set zip file name - encrypted_metadata_output = current_filename - - # Set context for the subject - current_date_str = datetime.now().strftime("%Y-%m-%d") - - # Encrypt the metadata - encrypted_metadata_in_bytes = self._encrypt_metadata( - s3_client.get_file(file_name), encrypted_metadata_output, str.encode(settings.SECRET_ENCRYPT_METADATA) - ) + encrypted_zip_in_bytes = self.get_encrypted_zip(zip_filename, reports, password) # Send the metadata file that is zipped send_templated_email( template_name="emails/metadata_body.txt", subject_template_name="emails/metadata_subject.txt", - to_address=settings.DEFAULT_FROM_EMAIL, - context={"current_date_str": current_date_str}, - attachment_file=encrypted_metadata_in_bytes, + to_address=self.email_to, + context={"current_date_str": datetime.now().strftime("%Y-%m-%d")}, + attachment_file=encrypted_zip_in_bytes, ) - def _encrypt_metadata(self, input_file, output_file, password): + + def get_encrypted_zip(self, zip_filename, reports, password): """Helper function for encrypting the attachment file""" - current_date = datetime.now().strftime("%m%d%Y") - current_filename = f"domain-metadata-{current_date}.csv" + # Using ZIP_DEFLATED bc it's a more common compression method supported by most zip utilities and faster # We could also use compression=pyzipper.ZIP_LZMA if we are looking for smaller file size with pyzipper.AESZipFile( - output_file, "w", compression=pyzipper.ZIP_DEFLATED, encryption=pyzipper.WZ_AES + zip_filename, "w", compression=pyzipper.ZIP_DEFLATED, encryption=pyzipper.WZ_AES ) as f_out: - f_out.setpassword(password) - f_out.writestr(current_filename, input_file) - with open(output_file, "rb") as file_data: + f_out.setpassword(str.encode(password)) + for report_name, report_value in reports.items(): + report_filename = report_value["report_filename"] + report_function = report_value["report_function"] + + report = self.write_and_return_report(report_function) + f_out.writestr(report_filename, report) + logger.info(f"Generated {report_name}") + + # Get the final report for emailing purposes + with open(zip_filename, "rb") as file_data: attachment_in_bytes = file_data.read() + return attachment_in_bytes + + def write_and_return_report(self, report_function): + """Writes a report to a StringIO object given a report_function and returns the string.""" + report_bytes = StringIO() + report_function(report_bytes) + + # Rewind the buffer to the beginning after writing + report_bytes.seek(0) + return report_bytes.read() From de6ff1d7d1f1115d5bf1a93212e3818aa8da2dae Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 17 Jun 2024 14:18:33 -0600 Subject: [PATCH 02/21] Streamline --- .../commands/email_current_metadata_report.py | 30 +++++++------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/src/registrar/management/commands/email_current_metadata_report.py b/src/registrar/management/commands/email_current_metadata_report.py index 1e8faac82..3f38bec31 100644 --- a/src/registrar/management/commands/email_current_metadata_report.py +++ b/src/registrar/management/commands/email_current_metadata_report.py @@ -23,7 +23,6 @@ class Command(BaseCommand): "which is based off of all existing Domains." ) current_date = datetime.now().strftime("%m%d%Y") - email_to: str def add_arguments(self, parser): """Add our two filename arguments.""" @@ -35,31 +34,27 @@ class Command(BaseCommand): def handle(self, **options): """Grabs the directory then creates domain-metadata.csv in that directory""" - self.email_to = options.get("emailTo") + zip_filename = f"domain-metadata-{self.current_date}.zip" + email_to = options.get("emailTo") # Don't email to DEFAULT_FROM_EMAIL when not prod. - if not settings.IS_PRODUCTION and self.email_to == settings.DEFAULT_FROM_EMAIL: + if not settings.IS_PRODUCTION and email_to == settings.DEFAULT_FROM_EMAIL: raise ValueError( "The --emailTo arg must be specified in non-prod environments, " "and the arg must not equal the DEFAULT_FROM_EMAIL value (aka: help@get.gov)." ) logger.info("Generating report...") - zip_filename = f"domain-metadata-{self.current_date}.zip" try: - self.email_current_metadata_report(zip_filename) + self.email_current_metadata_report(zip_filename, email_to) except Exception as err: # TODO - #1317: Notify operations when auto report generation fails raise err else: logger.info(f"Success! Created {zip_filename} and successfully sent out an email!") - def email_current_metadata_report(self, zip_filename): - """Creates a current-metadata.csv file under the specified directory, - then uploads it to a AWS S3 bucket. This is done for resiliency - reasons in the event our application goes down and/or the email - cannot send -- we'll still be able to grab info from the S3 - instance""" + def email_current_metadata_report(self, zip_filename, email_to): + """Emails a password protected zip containing domain-metadata and domain-request-metadata""" reports = { "Domain report": { "report_filename": f"domain-metadata-{self.current_date}.csv", @@ -81,7 +76,7 @@ class Command(BaseCommand): send_templated_email( template_name="emails/metadata_body.txt", subject_template_name="emails/metadata_subject.txt", - to_address=self.email_to, + to_address=email_to, context={"current_date_str": datetime.now().strftime("%Y-%m-%d")}, attachment_file=encrypted_zip_in_bytes, ) @@ -96,13 +91,10 @@ class Command(BaseCommand): zip_filename, "w", compression=pyzipper.ZIP_DEFLATED, encryption=pyzipper.WZ_AES ) as f_out: f_out.setpassword(str.encode(password)) - for report_name, report_value in reports.items(): - report_filename = report_value["report_filename"] - report_function = report_value["report_function"] - - report = self.write_and_return_report(report_function) - f_out.writestr(report_filename, report) - logger.info(f"Generated {report_name}") + for report_name, report in reports.items(): + logger.info(f"Generating {report_name}") + report = self.write_and_return_report(report["report_function"]) + f_out.writestr(report["report_filename"], report) # Get the final report for emailing purposes with open(zip_filename, "rb") as file_data: From a64ecbd0d20f82d5136bd30f905ef9caedcdecdf Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 17 Jun 2024 14:22:10 -0600 Subject: [PATCH 03/21] Lint --- .../management/commands/email_current_metadata_report.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/management/commands/email_current_metadata_report.py b/src/registrar/management/commands/email_current_metadata_report.py index 3f38bec31..b76e77608 100644 --- a/src/registrar/management/commands/email_current_metadata_report.py +++ b/src/registrar/management/commands/email_current_metadata_report.py @@ -18,6 +18,7 @@ logger = logging.getLogger(__name__) class Command(BaseCommand): """Emails a encrypted zip file containing a csv of our domains and domain requests""" + help = ( "Generates and uploads a domain-metadata.csv file to our S3 bucket " "which is based off of all existing Domains." @@ -65,6 +66,7 @@ class Command(BaseCommand): "report_function": csv_export.DomainRequestExport.export_full_domain_request_report, }, } + # Set the password equal to our content in SECRET_ENCRYPT_METADATA. # For local development, this will be "devpwd" unless otherwise set. override = settings.SECRET_ENCRYPT_METADATA is None and not settings.IS_PRODUCTION @@ -81,7 +83,6 @@ class Command(BaseCommand): attachment_file=encrypted_zip_in_bytes, ) - def get_encrypted_zip(self, zip_filename, reports, password): """Helper function for encrypting the attachment file""" From c020007d515cfdd87f9eb788234f8dd14bec5f9a Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 19 Jun 2024 07:30:09 -0400 Subject: [PATCH 04/21] wip --- src/registrar/registrar_middleware.py | 49 +++++++++++++++++---------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 79e3b7a11..7ba304aba 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -52,6 +52,30 @@ class CheckUserProfileMiddleware: "/admin", ] + def __init__(self): + self.setup_page = reverse("finish-user-profile-setup") + self.profile_page = reverse("user-profile") + self.logout_page = reverse("logout") + + self.regular_excluded_pages = [ + self.setup_page, + self.logout_page, + "/admin", + ] + self.other_excluded_pages = [ + self.profile_page, + self.logout_page, + "/admin", + ] + + self.excluded_pages = { + self.setup_page: self.regular_excluded_pages, + self.profile_page: self.other_excluded_pages + } + + def _get_excluded_pages(self, page): + return self.excluded_pages.get(page, []) + def __call__(self, request): response = self.get_response(request) return response @@ -68,16 +92,16 @@ class CheckUserProfileMiddleware: return None if request.user.is_authenticated: + profile_page = self.profile_page + if request.user.verification_type == User.VerificationTypeChoices.REGULAR: + profile_page = self.setup_page if hasattr(request.user, "finished_setup") and not request.user.finished_setup: - if request.user.verification_type == User.VerificationTypeChoices.REGULAR: - return self._handle_regular_user_setup_not_finished(request) - else: - return self._handle_other_user_setup_not_finished(request) + return self._handle_user_setup_not_finished(request, profile_page) # Continue processing the view return None - def _handle_regular_user_setup_not_finished(self, request): + def _handle_user_setup_not_finished(self, request, profile_page): """Redirects the given user to the finish setup page. We set the "redirect" query param equal to where the user wants to go. @@ -93,7 +117,8 @@ class CheckUserProfileMiddleware: custom_redirect = "domain-request:" if request.path == "/request/" else None # Don't redirect on excluded pages (such as the setup page itself) - if not any(request.path.startswith(page) for page in self.regular_excluded_pages): + #if not any(request.path.startswith(page) for page in excluded_pages): + if not any(request.path.startswith(page) for page in self._get_excluded_pages(profile_page)): # Preserve the original query parameters, and coerce them into a dict query_params = parse_qs(request.META["QUERY_STRING"]) @@ -103,19 +128,9 @@ class CheckUserProfileMiddleware: query_params["redirect"] = custom_redirect # Add our new query param, while preserving old ones - new_setup_page = replace_url_queryparams(self.setup_page, query_params) if query_params else self.setup_page + new_setup_page = replace_url_queryparams(profile_page, query_params) if query_params else profile_page return HttpResponseRedirect(new_setup_page) else: # Process the view as normal return None - - def _handle_other_user_setup_not_finished(self, request): - """Redirects the given user to the profile page to finish setup.""" - - # Don't redirect on excluded pages (such as the setup page itself) - if not any(request.path.startswith(page) for page in self.other_excluded_pages): - return HttpResponseRedirect(self.profile_page) - else: - # Process the view as normal - return None From a6c4710777962502b15a5f1744695df17fb056da Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 19 Jun 2024 14:26:36 -0400 Subject: [PATCH 05/21] minimally works, requires cleanup --- src/registrar/forms/user_profile.py | 2 + src/registrar/registrar_middleware.py | 15 -- .../templates/domain_request_intro.html | 2 +- .../includes/finish_profile_form.html | 2 + .../templates/includes/profile_form.html | 3 + src/registrar/templates/profile.html | 8 +- src/registrar/views/user_profile.py | 195 +++++++++++------- 7 files changed, 130 insertions(+), 97 deletions(-) diff --git a/src/registrar/forms/user_profile.py b/src/registrar/forms/user_profile.py index 3dd8cbdce..682e1a5df 100644 --- a/src/registrar/forms/user_profile.py +++ b/src/registrar/forms/user_profile.py @@ -10,6 +10,8 @@ from registrar.models.utility.domain_helper import DomainHelper class UserProfileForm(forms.ModelForm): """Form for updating user profile.""" + redirect = forms.CharField(widget=forms.HiddenInput(), required=False) + class Meta: model = Contact fields = ["first_name", "middle_name", "last_name", "title", "email", "phone"] diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 7ba304aba..88eb9af2d 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -38,21 +38,6 @@ class CheckUserProfileMiddleware: def __init__(self, get_response): self.get_response = get_response - self.setup_page = reverse("finish-user-profile-setup") - self.profile_page = reverse("user-profile") - self.logout_page = reverse("logout") - self.regular_excluded_pages = [ - self.setup_page, - self.logout_page, - "/admin", - ] - self.other_excluded_pages = [ - self.profile_page, - self.logout_page, - "/admin", - ] - - def __init__(self): self.setup_page = reverse("finish-user-profile-setup") self.profile_page = reverse("user-profile") self.logout_page = reverse("logout") diff --git a/src/registrar/templates/domain_request_intro.html b/src/registrar/templates/domain_request_intro.html index 2bfeeeef1..370ea2b2b 100644 --- a/src/registrar/templates/domain_request_intro.html +++ b/src/registrar/templates/domain_request_intro.html @@ -18,7 +18,7 @@ completing your domain request might take around 15 minutes.

{% if has_profile_feature_flag %}

How we’ll reach you

-

While reviewing your domain request, we may need to reach out with questions. We’ll also email you when we complete our review If the contact information below is not correct, visit your profile to make updates.

+

While reviewing your domain request, we may need to reach out with questions. We’ll also email you when we complete our review If the contact information below is not correct, visit your profile to make updates.

{% include "includes/profile_information.html" with user=user%} {% endif %} diff --git a/src/registrar/templates/includes/finish_profile_form.html b/src/registrar/templates/includes/finish_profile_form.html index a40534b48..1e48d5578 100644 --- a/src/registrar/templates/includes/finish_profile_form.html +++ b/src/registrar/templates/includes/finish_profile_form.html @@ -33,6 +33,8 @@ Your contact information + + {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable usa-form-editable--no-border padding-top-2" %} {% input_with_errors form.full_name %} {% endwith %} diff --git a/src/registrar/templates/includes/profile_form.html b/src/registrar/templates/includes/profile_form.html index cb3e734bf..966b92b01 100644 --- a/src/registrar/templates/includes/profile_form.html +++ b/src/registrar/templates/includes/profile_form.html @@ -19,6 +19,9 @@
{% csrf_token %} + {# Include the hidden 'redirect' field #} + + {% input_with_errors form.first_name %} {% input_with_errors form.middle_name %} diff --git a/src/registrar/templates/profile.html b/src/registrar/templates/profile.html index c62d0a7c1..41471fe88 100644 --- a/src/registrar/templates/profile.html +++ b/src/registrar/templates/profile.html @@ -25,19 +25,13 @@ Edit your User Profile | {% include "includes/form_errors.html" with form=form %} {% if show_back_button %} - + - {% if not return_to_request %}

{{ profile_back_button_text }}

- {% else %} -

- Go back to your domain request -

- {% endif %} {% endif %} diff --git a/src/registrar/views/user_profile.py b/src/registrar/views/user_profile.py index f148f5652..ddb6136f5 100644 --- a/src/registrar/views/user_profile.py +++ b/src/registrar/views/user_profile.py @@ -9,6 +9,7 @@ from urllib.parse import parse_qs, unquote from urllib.parse import quote from django.contrib import messages +from django.http import QueryDict from django.views.generic.edit import FormMixin from registrar.forms.user_profile import UserProfileForm, FinishSetupProfileForm from django.urls import NoReverseMatch, reverse @@ -31,16 +32,48 @@ class UserProfileView(UserProfilePermissionView, FormMixin): Base View for the User Profile. Handles getting and setting the User Profile """ + class RedirectType(Enum): + """ + Enums for each type of redirection. Enforces behaviour on `get_redirect_url()`. + + - HOME: We want to redirect to reverse("home") + - BACK_TO_SELF: We want to redirect back to this page + - TO_SPECIFIC_PAGE: We want to redirect to the page specified in the queryparam "redirect" + - COMPLETE_SETUP: Indicates that we want to navigate BACK_TO_SELF, but the subsequent + redirect after the next POST should be either HOME or TO_SPECIFIC_PAGE + """ + + HOME = "home" + TO_SPECIFIC_PAGE = "domain_request" + BACK_TO_SELF = "back_to_self" + COMPLETE_SETUP = "complete_setup" + + @classmethod + def get_all_redirect_types(cls) -> list[str]: + """Returns the value of every redirect type defined in this enum.""" + return [r.value for r in cls] + model = Contact template_name = "profile.html" form_class = UserProfileForm + base_view_name = "user-profile" + + all_redirect_types = RedirectType.get_all_redirect_types() + redirect_type: RedirectType def get(self, request, *args, **kwargs): """Handle get requests by getting user's contact object and setting object and form to context before rendering.""" - self._refresh_session_and_object(request) - form = self.form_class(instance=self.object) - context = self.get_context_data(object=self.object, form=form) + #self._refresh_session_and_object(request) + self.object = self.get_object() + + # Get the redirect parameter from the query string + redirect = request.GET.get('redirect', 'home') + + logger.info(f"redirect value is {redirect}") + + form = self.form_class(instance=self.object, initial={'redirect': redirect}) + context = self.get_context_data(object=self.object, form=form, redirect=redirect) if ( hasattr(self.user, "finished_setup") @@ -63,8 +96,8 @@ class UserProfileView(UserProfilePermissionView, FormMixin): @waffle_flag("profile_feature") # type: ignore def dispatch(self, request, *args, **kwargs): # type: ignore # Store the original queryparams to persist them - query_params = request.META["QUERY_STRING"] - request.session["query_params"] = query_params + # query_params = request.META["QUERY_STRING"] + # request.session["query_params"] = query_params return super().dispatch(request, *args, **kwargs) def get_context_data(self, **kwargs): @@ -73,10 +106,14 @@ class UserProfileView(UserProfilePermissionView, FormMixin): # This is a django waffle flag which toggles features based off of the "flag" table context["has_profile_feature_flag"] = flag_is_active(self.request, "profile_feature") - # The text for the back button on this page - context["profile_back_button_text"] = "Go to manage your domains" - context["show_back_button"] = False + # Set the profile_back_button_text based on the redirect parameter + if kwargs.get('redirect') == 'domain-request:': + context["profile_back_button_text"] = "Go back to your request" + else: + context["profile_back_button_text"] = "Go to manage your domains" + # Show back button conditional on user having finished setup + context["show_back_button"] = False if hasattr(self.user, "finished_setup") and self.user.finished_setup: context["user_finished_setup"] = True context["show_back_button"] = True @@ -84,21 +121,30 @@ class UserProfileView(UserProfilePermissionView, FormMixin): return context def get_success_url(self): - """Redirect to the user's profile page.""" + """Redirect to the user's profile page with updated query parameters.""" - query_params = {} - if "query_params" in self.session: - params = unquote(self.session["query_params"]) - query_params = parse_qs(params) + # Get the redirect parameter from the form submission + redirect_param = self.request.POST.get('redirect', None) - # Preserve queryparams and add them back to the url - base_url = reverse("user-profile") - new_redirect = replace_url_queryparams(base_url, query_params, convert_list_to_csv=True) - return new_redirect + # Initialize QueryDict with existing query parameters from current request + query_params = QueryDict(mutable=True) + query_params.update(self.request.GET) + + # Update query parameters with the 'redirect' value from form submission + if redirect_param and redirect_param != 'home': + query_params['redirect'] = redirect_param + + # Generate the URL with updated query parameters + base_url = reverse(self.base_view_name) + + # Generate the full url from the given query params + full_url = replace_url_queryparams(base_url, query_params) + return full_url def post(self, request, *args, **kwargs): """Handle post requests (form submissions)""" - self._refresh_session_and_object(request) + #self._refresh_session_and_object(request) + self.object = self.get_object() form = self.form_class(request.POST, instance=self.object) if form.is_valid(): @@ -133,80 +179,75 @@ class FinishProfileSetupView(UserProfileView): """This view forces the user into providing additional details that we may have missed from Login.gov""" - class RedirectType(Enum): - """ - Enums for each type of redirection. Enforces behaviour on `get_redirect_url()`. - - - HOME: We want to redirect to reverse("home") - - BACK_TO_SELF: We want to redirect back to this page - - TO_SPECIFIC_PAGE: We want to redirect to the page specified in the queryparam "redirect" - - COMPLETE_SETUP: Indicates that we want to navigate BACK_TO_SELF, but the subsequent - redirect after the next POST should be either HOME or TO_SPECIFIC_PAGE - """ - - HOME = "home" - TO_SPECIFIC_PAGE = "domain_request" - BACK_TO_SELF = "back_to_self" - COMPLETE_SETUP = "complete_setup" - - @classmethod - def get_all_redirect_types(cls) -> list[str]: - """Returns the value of every redirect type defined in this enum.""" - return [r.value for r in cls] - template_name = "finish_profile_setup.html" form_class = FinishSetupProfileForm model = Contact - all_redirect_types = RedirectType.get_all_redirect_types() - redirect_type: RedirectType + base_view_name = "finish-user-profile-setup" def get_context_data(self, **kwargs): - + """Extend get_context_data to include has_profile_feature_flag""" context = super().get_context_data(**kwargs) - # Hide the back button by default + # Show back button conditional on user having finished setup context["show_back_button"] = False - - if self.redirect_type == self.RedirectType.COMPLETE_SETUP: + if hasattr(self.user, "finished_setup") and self.user.finished_setup: context["confirm_changes"] = True - - if "redirect_viewname" not in self.session: + if kwargs.get('redirect') == 'home': + context["profile_back_button_text"] = "Go to manage your domains" context["show_back_button"] = True else: context["going_to_specific_page"] = True context["redirect_button_text"] = "Continue to your request" - return context + + # def get_context_data(self, **kwargs): - @method_decorator(csrf_protect) - def dispatch(self, request, *args, **kwargs): - """ - Handles dispatching of the view, applying CSRF protection and checking the 'profile_feature' flag. + # context = super().get_context_data(**kwargs) - This method sets the redirect type based on the 'redirect' query parameter, - defaulting to BACK_TO_SELF if not provided. - It updates the session with the redirect view name if the redirect type is TO_SPECIFIC_PAGE. + # # Hide the back button by default + # context["show_back_button"] = False - Returns: - HttpResponse: The response generated by the parent class's dispatch method. - """ + # logger.info(f"self.redirect_type = {self.redirect_type}") + # if self.redirect_type == self.RedirectType.COMPLETE_SETUP: + # context["confirm_changes"] = True - # Update redirect type based on the query parameter if present - default_redirect_value = self.RedirectType.BACK_TO_SELF.value - redirect_value = request.GET.get("redirect", default_redirect_value) + # if "redirect_viewname" not in self.session: + # context["show_back_button"] = True + # else: + # context["going_to_specific_page"] = True + # context["redirect_button_text"] = "Continue to your request" - if redirect_value in self.all_redirect_types: - # If the redirect value is a preexisting value in our enum, set it to that. - self.redirect_type = self.RedirectType(redirect_value) - else: - # If the redirect type is undefined, then we assume that we are specifying a particular page to redirect to. - self.redirect_type = self.RedirectType.TO_SPECIFIC_PAGE + # return context - # Store the page that we want to redirect to for later use - request.session["redirect_viewname"] = str(redirect_value) + # @method_decorator(csrf_protect) + # def dispatch(self, request, *args, **kwargs): + # """ + # Handles dispatching of the view, applying CSRF protection and checking the 'profile_feature' flag. - return super().dispatch(request, *args, **kwargs) + # This method sets the redirect type based on the 'redirect' query parameter, + # defaulting to BACK_TO_SELF if not provided. + # It updates the session with the redirect view name if the redirect type is TO_SPECIFIC_PAGE. + + # Returns: + # HttpResponse: The response generated by the parent class's dispatch method. + # """ + + # # Update redirect type based on the query parameter if present + # default_redirect_value = self.RedirectType.BACK_TO_SELF.value + # redirect_value = request.GET.get("redirect", default_redirect_value) + + # if redirect_value in self.all_redirect_types: + # # If the redirect value is a preexisting value in our enum, set it to that. + # self.redirect_type = self.RedirectType(redirect_value) + # else: + # # If the redirect type is undefined, then we assume that we are specifying a particular page to redirect to. + # self.redirect_type = self.RedirectType.TO_SPECIFIC_PAGE + + # # Store the page that we want to redirect to for later use + # request.session["redirect_viewname"] = str(redirect_value) + + # return super().dispatch(request, *args, **kwargs) def post(self, request, *args, **kwargs): """Form submission posts to this view.""" @@ -217,17 +258,23 @@ class FinishProfileSetupView(UserProfileView): if form.is_valid(): if "contact_setup_save_button" in request.POST: # Logic for when the 'Save' button is clicked - self.redirect_type = self.RedirectType.COMPLETE_SETUP + self.redirect_page = False + #self.redirect_type = self.RedirectType.COMPLETE_SETUP elif "contact_setup_submit_button" in request.POST: - specific_redirect = "redirect_viewname" in self.session - self.redirect_type = self.RedirectType.TO_SPECIFIC_PAGE if specific_redirect else self.RedirectType.HOME + self.redirect_page = True + # specific_redirect = "redirect_viewname" in self.session + # self.redirect_type = self.RedirectType.TO_SPECIFIC_PAGE if specific_redirect else self.RedirectType.HOME return self.form_valid(form) else: return self.form_invalid(form) def get_success_url(self): """Redirect to the nameservers page for the domain.""" - return self.get_redirect_url() + # Get the redirect parameter from the form submission + redirect_param = self.request.POST.get('redirect', None) + if self.redirect_page and redirect_param: + return reverse(redirect_param) + return super().get_success_url() def get_redirect_url(self): """ From f2e0875091ac37753f6b80fd0ec89322a3180416 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 19 Jun 2024 14:47:52 -0400 Subject: [PATCH 06/21] cleaned up code and formatted for readability --- src/registrar/registrar_middleware.py | 10 +- .../templates/finish_profile_setup.html | 4 +- .../includes/finish_profile_form.html | 4 +- src/registrar/views/user_profile.py | 171 ++---------------- 4 files changed, 27 insertions(+), 162 deletions(-) diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 88eb9af2d..be787aca9 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -41,7 +41,7 @@ class CheckUserProfileMiddleware: self.setup_page = reverse("finish-user-profile-setup") self.profile_page = reverse("user-profile") self.logout_page = reverse("logout") - + self.regular_excluded_pages = [ self.setup_page, self.logout_page, @@ -52,12 +52,12 @@ class CheckUserProfileMiddleware: self.logout_page, "/admin", ] - + self.excluded_pages = { self.setup_page: self.regular_excluded_pages, - self.profile_page: self.other_excluded_pages + self.profile_page: self.other_excluded_pages, } - + def _get_excluded_pages(self, page): return self.excluded_pages.get(page, []) @@ -102,7 +102,7 @@ class CheckUserProfileMiddleware: custom_redirect = "domain-request:" if request.path == "/request/" else None # Don't redirect on excluded pages (such as the setup page itself) - #if not any(request.path.startswith(page) for page in excluded_pages): + # if not any(request.path.startswith(page) for page in excluded_pages): if not any(request.path.startswith(page) for page in self._get_excluded_pages(profile_page)): # Preserve the original query parameters, and coerce them into a dict diff --git a/src/registrar/templates/finish_profile_setup.html b/src/registrar/templates/finish_profile_setup.html index f8070551b..6e35ad5da 100644 --- a/src/registrar/templates/finish_profile_setup.html +++ b/src/registrar/templates/finish_profile_setup.html @@ -5,7 +5,7 @@ {# Disable the redirect #} {% block logo %} - {% include "includes/gov_extended_logo.html" with logo_clickable=confirm_changes %} + {% include "includes/gov_extended_logo.html" with logo_clickable=user_finished_setup %} {% endblock %} {# Add the new form #} @@ -16,5 +16,5 @@ {% endblock content_bottom %} {% block footer %} - {% include "includes/footer.html" with show_manage_your_domains=confirm_changes %} + {% include "includes/footer.html" with show_manage_your_domains=user_finished_setup %} {% endblock footer %} diff --git a/src/registrar/templates/includes/finish_profile_form.html b/src/registrar/templates/includes/finish_profile_form.html index 1e48d5578..d43ff812c 100644 --- a/src/registrar/templates/includes/finish_profile_form.html +++ b/src/registrar/templates/includes/finish_profile_form.html @@ -34,7 +34,7 @@ - + {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable usa-form-editable--no-border padding-top-2" %} {% input_with_errors form.full_name %} {% endwith %} @@ -80,7 +80,7 @@ - {% if confirm_changes and going_to_specific_page %} + {% if user_finished_setup and going_to_specific_page %} diff --git a/src/registrar/views/user_profile.py b/src/registrar/views/user_profile.py index ddb6136f5..939f11933 100644 --- a/src/registrar/views/user_profile.py +++ b/src/registrar/views/user_profile.py @@ -2,11 +2,7 @@ """ -from enum import Enum import logging -from urllib.parse import parse_qs, unquote - -from urllib.parse import quote from django.contrib import messages from django.http import QueryDict @@ -21,9 +17,6 @@ from registrar.models.utility.generic_helper import replace_url_queryparams from registrar.views.utility.permission_views import UserProfilePermissionView from waffle.decorators import flag_is_active, waffle_flag -from django.utils.decorators import method_decorator -from django.views.decorators.csrf import csrf_protect - logger = logging.getLogger(__name__) @@ -32,47 +25,20 @@ class UserProfileView(UserProfilePermissionView, FormMixin): Base View for the User Profile. Handles getting and setting the User Profile """ - class RedirectType(Enum): - """ - Enums for each type of redirection. Enforces behaviour on `get_redirect_url()`. - - - HOME: We want to redirect to reverse("home") - - BACK_TO_SELF: We want to redirect back to this page - - TO_SPECIFIC_PAGE: We want to redirect to the page specified in the queryparam "redirect" - - COMPLETE_SETUP: Indicates that we want to navigate BACK_TO_SELF, but the subsequent - redirect after the next POST should be either HOME or TO_SPECIFIC_PAGE - """ - - HOME = "home" - TO_SPECIFIC_PAGE = "domain_request" - BACK_TO_SELF = "back_to_self" - COMPLETE_SETUP = "complete_setup" - - @classmethod - def get_all_redirect_types(cls) -> list[str]: - """Returns the value of every redirect type defined in this enum.""" - return [r.value for r in cls] - model = Contact template_name = "profile.html" form_class = UserProfileForm base_view_name = "user-profile" - all_redirect_types = RedirectType.get_all_redirect_types() - redirect_type: RedirectType - def get(self, request, *args, **kwargs): """Handle get requests by getting user's contact object and setting object and form to context before rendering.""" - #self._refresh_session_and_object(request) self.object = self.get_object() # Get the redirect parameter from the query string - redirect = request.GET.get('redirect', 'home') + redirect = request.GET.get("redirect", "home") - logger.info(f"redirect value is {redirect}") - - form = self.form_class(instance=self.object, initial={'redirect': redirect}) + form = self.form_class(instance=self.object, initial={"redirect": redirect}) context = self.get_context_data(object=self.object, form=form, redirect=redirect) if ( @@ -82,22 +48,10 @@ class UserProfileView(UserProfilePermissionView, FormMixin): ): context["show_confirmation_modal"] = True - return_to_request = request.GET.get("return_to_request") - if return_to_request: - context["return_to_request"] = True - return self.render_to_response(context) - def _refresh_session_and_object(self, request): - """Sets the current session to self.session and the current object to self.object""" - self.session = request.session - self.object = self.get_object() - @waffle_flag("profile_feature") # type: ignore def dispatch(self, request, *args, **kwargs): # type: ignore - # Store the original queryparams to persist them - # query_params = request.META["QUERY_STRING"] - # request.session["query_params"] = query_params return super().dispatch(request, *args, **kwargs) def get_context_data(self, **kwargs): @@ -107,7 +61,7 @@ class UserProfileView(UserProfilePermissionView, FormMixin): context["has_profile_feature_flag"] = flag_is_active(self.request, "profile_feature") # Set the profile_back_button_text based on the redirect parameter - if kwargs.get('redirect') == 'domain-request:': + if kwargs.get("redirect") == "domain-request:": context["profile_back_button_text"] = "Go back to your request" else: context["profile_back_button_text"] = "Go to manage your domains" @@ -124,15 +78,15 @@ class UserProfileView(UserProfilePermissionView, FormMixin): """Redirect to the user's profile page with updated query parameters.""" # Get the redirect parameter from the form submission - redirect_param = self.request.POST.get('redirect', None) + redirect_param = self.request.POST.get("redirect", None) # Initialize QueryDict with existing query parameters from current request query_params = QueryDict(mutable=True) query_params.update(self.request.GET) # Update query parameters with the 'redirect' value from form submission - if redirect_param and redirect_param != 'home': - query_params['redirect'] = redirect_param + if redirect_param and redirect_param != "home": + query_params["redirect"] = redirect_param # Generate the URL with updated query parameters base_url = reverse(self.base_view_name) @@ -143,7 +97,6 @@ class UserProfileView(UserProfilePermissionView, FormMixin): def post(self, request, *args, **kwargs): """Handle post requests (form submissions)""" - #self._refresh_session_and_object(request) self.object = self.get_object() form = self.form_class(request.POST, instance=self.object) @@ -192,127 +145,39 @@ class FinishProfileSetupView(UserProfileView): # Show back button conditional on user having finished setup context["show_back_button"] = False if hasattr(self.user, "finished_setup") and self.user.finished_setup: - context["confirm_changes"] = True - if kwargs.get('redirect') == 'home': - context["profile_back_button_text"] = "Go to manage your domains" + if kwargs.get("redirect") == "home": context["show_back_button"] = True else: context["going_to_specific_page"] = True context["redirect_button_text"] = "Continue to your request" return context - - # def get_context_data(self, **kwargs): - - # context = super().get_context_data(**kwargs) - - # # Hide the back button by default - # context["show_back_button"] = False - - # logger.info(f"self.redirect_type = {self.redirect_type}") - # if self.redirect_type == self.RedirectType.COMPLETE_SETUP: - # context["confirm_changes"] = True - - # if "redirect_viewname" not in self.session: - # context["show_back_button"] = True - # else: - # context["going_to_specific_page"] = True - # context["redirect_button_text"] = "Continue to your request" - - # return context - - # @method_decorator(csrf_protect) - # def dispatch(self, request, *args, **kwargs): - # """ - # Handles dispatching of the view, applying CSRF protection and checking the 'profile_feature' flag. - - # This method sets the redirect type based on the 'redirect' query parameter, - # defaulting to BACK_TO_SELF if not provided. - # It updates the session with the redirect view name if the redirect type is TO_SPECIFIC_PAGE. - - # Returns: - # HttpResponse: The response generated by the parent class's dispatch method. - # """ - - # # Update redirect type based on the query parameter if present - # default_redirect_value = self.RedirectType.BACK_TO_SELF.value - # redirect_value = request.GET.get("redirect", default_redirect_value) - - # if redirect_value in self.all_redirect_types: - # # If the redirect value is a preexisting value in our enum, set it to that. - # self.redirect_type = self.RedirectType(redirect_value) - # else: - # # If the redirect type is undefined, then we assume that we are specifying a particular page to redirect to. - # self.redirect_type = self.RedirectType.TO_SPECIFIC_PAGE - - # # Store the page that we want to redirect to for later use - # request.session["redirect_viewname"] = str(redirect_value) - - # return super().dispatch(request, *args, **kwargs) def post(self, request, *args, **kwargs): """Form submission posts to this view.""" - self._refresh_session_and_object(request) + self.object = self.get_object() form = self.form_class(request.POST, instance=self.object) # Get the current form and validate it if form.is_valid(): if "contact_setup_save_button" in request.POST: - # Logic for when the 'Save' button is clicked + # Logic for when the 'Save' button is clicked, which indicates + # user should stay on this page self.redirect_page = False - #self.redirect_type = self.RedirectType.COMPLETE_SETUP elif "contact_setup_submit_button" in request.POST: + # Logic for when the other button is clicked, which indicates + # the user should be taken to the redirect page self.redirect_page = True - # specific_redirect = "redirect_viewname" in self.session - # self.redirect_type = self.RedirectType.TO_SPECIFIC_PAGE if specific_redirect else self.RedirectType.HOME return self.form_valid(form) else: return self.form_invalid(form) def get_success_url(self): - """Redirect to the nameservers page for the domain.""" - # Get the redirect parameter from the form submission - redirect_param = self.request.POST.get('redirect', None) - if self.redirect_page and redirect_param: - return reverse(redirect_param) - return super().get_success_url() - - def get_redirect_url(self): - """ - Returns a URL string based on the current value of self.redirect_type. - - Depending on self.redirect_type, constructs a base URL and appends a - 'redirect' query parameter. Handles different redirection types such as - HOME, BACK_TO_SELF, COMPLETE_SETUP, and TO_SPECIFIC_PAGE. - - Returns: - str: The full URL with the appropriate query parameters. - """ - - # These redirect types redirect to the same page - self_redirect = [self.RedirectType.BACK_TO_SELF, self.RedirectType.COMPLETE_SETUP] - - # Maps the redirect type to a URL - base_url = "" + """Redirect to the redirect page, or redirect to the current page""" try: - if self.redirect_type in self_redirect: - base_url = reverse("finish-user-profile-setup") - elif self.redirect_type == self.RedirectType.TO_SPECIFIC_PAGE: - # We only allow this session value to use viewnames, - # because this restricts what can be redirected to. - desired_view = self.session["redirect_viewname"] - self.session.pop("redirect_viewname") - base_url = reverse(desired_view) - else: - base_url = reverse("home") + # Get the redirect parameter from the form submission + redirect_param = self.request.POST.get("redirect", None) + if self.redirect_page and redirect_param: + return reverse(redirect_param) except NoReverseMatch as err: logger.error(f"get_redirect_url -> Could not find the specified page. Err: {err}") - - query_params = {} - - # Quote cleans up the value so that it can be used in a url - if self.redirect_type and self.redirect_type.value: - query_params["redirect"] = quote(self.redirect_type.value) - - # Generate the full url from the given query params - full_url = replace_url_queryparams(base_url, query_params) - return full_url + return super().get_success_url() From 2ced462f94bb88b3caa48e004ff4e62fdcdf28fc Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 19 Jun 2024 15:24:31 -0400 Subject: [PATCH 07/21] wip --- src/registrar/views/user_profile.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/views/user_profile.py b/src/registrar/views/user_profile.py index 939f11933..865ed2b78 100644 --- a/src/registrar/views/user_profile.py +++ b/src/registrar/views/user_profile.py @@ -159,6 +159,7 @@ class FinishProfileSetupView(UserProfileView): # Get the current form and validate it if form.is_valid(): + self.redirect_page = True if "contact_setup_save_button" in request.POST: # Logic for when the 'Save' button is clicked, which indicates # user should stay on this page From e0df3ed4537e68bbe620a1d97c8d98f7ff56c50a Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 19 Jun 2024 16:02:57 -0400 Subject: [PATCH 08/21] updated tests --- src/registrar/tests/test_views.py | 16 +++++++++++++--- src/registrar/views/user_profile.py | 4 ++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index fa63da17a..c2ea2e6f4 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -530,8 +530,11 @@ class FinishUserProfileTests(TestWithUser, WebTest): session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - def _submit_form_webtest(self, form, follow=False): - page = form.submit() + def _submit_form_webtest(self, form, follow=False, name=None): + if name: + page = form.submit(name=name) + else: + page = form.submit() self._set_session_cookie() return page.follow() if follow else page @@ -605,6 +608,13 @@ class FinishUserProfileTests(TestWithUser, WebTest): self.assertEqual(completed_setup_page.status_code, 200) + finish_setup_form = completed_setup_page.form + + # Submit the form using the specific submit button to execute the redirect + completed_setup_page = self._submit_form_webtest(finish_setup_form, follow=True, name="contact_setup_submit_button") + self.assertEqual(completed_setup_page.status_code, 200) + + # Assert that we are still on the # Assert that we're on the domain request page self.assertNotContains(completed_setup_page, "Finish setting up your profile") self.assertNotContains(completed_setup_page, "What contact information should we use to reach you?") @@ -822,7 +832,7 @@ class UserProfileTests(TestWithUser, WebTest): """tests user profile when profile_feature is on, and when they are redirected from the domain request page""" with override_flag("profile_feature", active=True): - response = self.client.get("/user-profile?return_to_request=True") + response = self.client.get("/user-profile?redirect=domain-request:") self.assertContains(response, "Your profile") self.assertContains(response, "Go back to your domain request") self.assertNotContains(response, "Back to manage your domains") diff --git a/src/registrar/views/user_profile.py b/src/registrar/views/user_profile.py index 865ed2b78..3f9aeb79f 100644 --- a/src/registrar/views/user_profile.py +++ b/src/registrar/views/user_profile.py @@ -62,7 +62,7 @@ class UserProfileView(UserProfilePermissionView, FormMixin): # Set the profile_back_button_text based on the redirect parameter if kwargs.get("redirect") == "domain-request:": - context["profile_back_button_text"] = "Go back to your request" + context["profile_back_button_text"] = "Go back to your domain request" else: context["profile_back_button_text"] = "Go to manage your domains" @@ -159,7 +159,7 @@ class FinishProfileSetupView(UserProfileView): # Get the current form and validate it if form.is_valid(): - self.redirect_page = True + self.redirect_page = False if "contact_setup_save_button" in request.POST: # Logic for when the 'Save' button is clicked, which indicates # user should stay on this page From 60f10cb3cd88947cb75fa0967fd5abd021af9f97 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 19 Jun 2024 16:05:14 -0400 Subject: [PATCH 09/21] updated code for readability --- src/registrar/tests/test_views.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index c2ea2e6f4..f719ba39f 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -611,10 +611,12 @@ class FinishUserProfileTests(TestWithUser, WebTest): finish_setup_form = completed_setup_page.form # Submit the form using the specific submit button to execute the redirect - completed_setup_page = self._submit_form_webtest(finish_setup_form, follow=True, name="contact_setup_submit_button") + completed_setup_page = self._submit_form_webtest( + finish_setup_form, follow=True, name="contact_setup_submit_button" + ) self.assertEqual(completed_setup_page.status_code, 200) - # Assert that we are still on the + # Assert that we are still on the # Assert that we're on the domain request page self.assertNotContains(completed_setup_page, "Finish setting up your profile") self.assertNotContains(completed_setup_page, "What contact information should we use to reach you?") From baee980b47e1b578ad7a1a554296ffca168b1faf Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 20 Jun 2024 17:48:00 -0400 Subject: [PATCH 10/21] cleaned up a comment --- src/registrar/registrar_middleware.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index be787aca9..e0ebb4dc5 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -102,7 +102,6 @@ class CheckUserProfileMiddleware: custom_redirect = "domain-request:" if request.path == "/request/" else None # Don't redirect on excluded pages (such as the setup page itself) - # if not any(request.path.startswith(page) for page in excluded_pages): if not any(request.path.startswith(page) for page in self._get_excluded_pages(profile_page)): # Preserve the original query parameters, and coerce them into a dict From 4ac9e9d4f3293a45628e3f906345c523f6d92f6b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 21 Jun 2024 10:06:31 -0600 Subject: [PATCH 11/21] Remove os --- .../management/commands/email_current_metadata_report.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/management/commands/email_current_metadata_report.py b/src/registrar/management/commands/email_current_metadata_report.py index b76e77608..0255cc178 100644 --- a/src/registrar/management/commands/email_current_metadata_report.py +++ b/src/registrar/management/commands/email_current_metadata_report.py @@ -1,7 +1,6 @@ """Generates current-metadata.csv then uploads to S3 + sends email""" import logging -import os import pyzipper from datetime import datetime From 5acaac7c6c7fe562731c733f50bc57d7b97753a9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 21 Jun 2024 10:19:00 -0600 Subject: [PATCH 12/21] Add some docs --- docs/operations/data_migration.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 472362a79..17aa9c606 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -697,3 +697,31 @@ Example: `cf ssh getgov-za` | | Parameter | Description | |:-:|:-------------------------- |:----------------------------------------------------------------------------| | 1 | **debug** | Increases logging detail. Defaults to False. | + +## Email current metadata report + +### Running on sandboxes + +#### Step 1: Login to CloudFoundry +```cf login -a api.fr.cloud.gov --sso``` + +#### Step 2: SSH into your environment +```cf ssh getgov-{space}``` + +Example: `cf ssh getgov-za` + +#### Step 3: Create a shell instance +```/tmp/lifecycle/shell``` + +#### Step 4: Running the script +```./manage.py email_current_metadata_report --emailTo {desired email address}``` + +### Running locally + +#### Step 1: Running the script +```docker-compose exec app ./manage.py email_current_metadata_report --emailTo {desired email address}``` + +##### Parameters +| | Parameter | Description | +|:-:|:-------------------------- |:-----------------------------------------------------------------------------------| +| 1 | **emailTo** | Specifies where the email will be emailed. Defaults to help@get.gov on production. | \ No newline at end of file From 3769113da3343865e7c1ae9693749c53180495bc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 24 Jun 2024 11:44:36 -0600 Subject: [PATCH 13/21] Fix bug --- .../management/commands/email_current_metadata_report.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/email_current_metadata_report.py b/src/registrar/management/commands/email_current_metadata_report.py index 0255cc178..595d39215 100644 --- a/src/registrar/management/commands/email_current_metadata_report.py +++ b/src/registrar/management/commands/email_current_metadata_report.py @@ -93,8 +93,8 @@ class Command(BaseCommand): f_out.setpassword(str.encode(password)) for report_name, report in reports.items(): logger.info(f"Generating {report_name}") - report = self.write_and_return_report(report["report_function"]) - f_out.writestr(report["report_filename"], report) + report_content = self.write_and_return_report(report["report_function"]) + f_out.writestr(report["report_filename"], report_content) # Get the final report for emailing purposes with open(zip_filename, "rb") as file_data: From aa1958c1432e4659443458294b805efeb9b66be2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 24 Jun 2024 13:11:08 -0600 Subject: [PATCH 14/21] Comment out password override for local testing --- .../management/commands/email_current_metadata_report.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/email_current_metadata_report.py b/src/registrar/management/commands/email_current_metadata_report.py index 595d39215..82b83ea9b 100644 --- a/src/registrar/management/commands/email_current_metadata_report.py +++ b/src/registrar/management/commands/email_current_metadata_report.py @@ -68,8 +68,10 @@ class Command(BaseCommand): # Set the password equal to our content in SECRET_ENCRYPT_METADATA. # For local development, this will be "devpwd" unless otherwise set. - override = settings.SECRET_ENCRYPT_METADATA is None and not settings.IS_PRODUCTION - password = "devpwd" if override else settings.SECRET_ENCRYPT_METADATA + # Uncomment these lines if you want to use this: + # override = settings.SECRET_ENCRYPT_METADATA is None and not settings.IS_PRODUCTION + # password = "devpwd" if override else settings.SECRET_ENCRYPT_METADATA + password = settings.SECRET_ENCRYPT_METADATA encrypted_zip_in_bytes = self.get_encrypted_zip(zip_filename, reports, password) From 5fe084cdabd1b98221192175b29d53f31fef6f30 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 24 Jun 2024 13:18:37 -0600 Subject: [PATCH 15/21] Add err handling --- .../management/commands/email_current_metadata_report.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/registrar/management/commands/email_current_metadata_report.py b/src/registrar/management/commands/email_current_metadata_report.py index 82b83ea9b..773199198 100644 --- a/src/registrar/management/commands/email_current_metadata_report.py +++ b/src/registrar/management/commands/email_current_metadata_report.py @@ -72,6 +72,10 @@ class Command(BaseCommand): # override = settings.SECRET_ENCRYPT_METADATA is None and not settings.IS_PRODUCTION # password = "devpwd" if override else settings.SECRET_ENCRYPT_METADATA password = settings.SECRET_ENCRYPT_METADATA + if not password: + raise ValueError( + "No password was specified for this zip file." + ) encrypted_zip_in_bytes = self.get_encrypted_zip(zip_filename, reports, password) From d5cd639b854552f8fba3d5201764c4795aa03a25 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 24 Jun 2024 13:21:42 -0600 Subject: [PATCH 16/21] Lint --- .../management/commands/email_current_metadata_report.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/registrar/management/commands/email_current_metadata_report.py b/src/registrar/management/commands/email_current_metadata_report.py index 773199198..905e4a57a 100644 --- a/src/registrar/management/commands/email_current_metadata_report.py +++ b/src/registrar/management/commands/email_current_metadata_report.py @@ -73,9 +73,7 @@ class Command(BaseCommand): # password = "devpwd" if override else settings.SECRET_ENCRYPT_METADATA password = settings.SECRET_ENCRYPT_METADATA if not password: - raise ValueError( - "No password was specified for this zip file." - ) + raise ValueError("No password was specified for this zip file.") encrypted_zip_in_bytes = self.get_encrypted_zip(zip_filename, reports, password) From 51e6a925daaa513230e71e7f16e5aa20d8166714 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 24 Jun 2024 15:39:05 -0400 Subject: [PATCH 17/21] formatted for code readability --- src/registrar/registrar_middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 1c2f63054..82ee9d0fc 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -121,7 +121,7 @@ class CheckUserProfileMiddleware: return HttpResponseRedirect(new_setup_page) else: # Process the view as normal - return None + return None class CheckPortfolioMiddleware: From 1c015b7aa1024622d0ad033bc0819346867d8944 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 24 Jun 2024 12:46:26 -0700 Subject: [PATCH 18/21] Update to have flag logic --- src/registrar/models/domain_request.py | 13 +++++++------ src/registrar/tests/test_models.py | 6 +++--- src/registrar/views/domain_request.py | 4 ++-- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 1c4725be1..49b84d472 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -17,6 +17,8 @@ from .utility.time_stamped_model import TimeStampedModel from ..utility.email import send_templated_email, EmailSendingError from itertools import chain +from waffle.decorators import flag_is_active + logger = logging.getLogger(__name__) @@ -1165,19 +1167,20 @@ class DomainRequest(TimeStampedModel): def _is_policy_acknowledgement_complete(self): return self.is_policy_acknowledged is not None - def _is_general_form_complete(self): + def _is_general_form_complete(self, request): + has_profile_feature_flag = flag_is_active(request, "profile_feature") return ( self._is_organization_name_and_address_complete() and self._is_authorizing_official_complete() and self._is_requested_domain_complete() and self._is_purpose_complete() - and self._is_submitter_complete() + and (self._is_submitter_complete() if not has_profile_feature_flag else True) and self._is_other_contacts_complete() and self._is_additional_details_complete() and self._is_policy_acknowledgement_complete() ) - def _form_complete(self): + def _form_complete(self, request): match self.generic_org_type: case DomainRequest.OrganizationChoices.FEDERAL: is_complete = self._is_federal_complete() @@ -1198,8 +1201,6 @@ class DomainRequest(TimeStampedModel): case _: # NOTE: Shouldn't happen, this is only if somehow they didn't choose an org type is_complete = False - - if not is_complete or not self._is_general_form_complete(): + if not is_complete or not self._is_general_form_complete(request): return False - return True diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 53af1674e..c3bb1fac9 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -2012,8 +2012,8 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.is_policy_acknowledged = None self.assertFalse(self.domain_request._is_policy_acknowledgement_complete()) - def test_form_complete(self): - self.assertTrue(self.domain_request._form_complete()) + def test_form_complete(self, request): + self.assertTrue(self.domain_request._form_complete(request)) self.domain_request.generic_org_type = None self.domain_request.save() - self.assertFalse(self.domain_request._form_complete()) + self.assertFalse(self.domain_request._form_complete(request)) diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index 77699e17a..73524dbe3 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -383,7 +383,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): has_profile_flag = flag_is_active(self.request, "profile_feature") context_stuff = {} - if DomainRequest._form_complete(self.domain_request): + if DomainRequest._form_complete(self.domain_request, self.request): modal_button = '" context_stuff = { "not_form": False, @@ -695,7 +695,7 @@ class Review(DomainRequestWizard): forms = [] # type: ignore def get_context_data(self): - if DomainRequest._form_complete(self.domain_request) is False: + if DomainRequest._form_complete(self.domain_request, self.request) is False: logger.warning("User arrived at review page with an incomplete form.") context = super().get_context_data() context["Step"] = Step.__members__ From 668611f13e1bc71f951ee97bbded0985f3ebad3f Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 24 Jun 2024 14:09:56 -0700 Subject: [PATCH 19/21] Update test --- src/registrar/tests/test_models.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index c3bb1fac9..272996147 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -3,6 +3,8 @@ from django.db.utils import IntegrityError from unittest.mock import patch from django.contrib.auth import get_user_model +from django.test import RequestFactory + from registrar.models import ( Contact, DomainRequest, @@ -1610,6 +1612,7 @@ class TestDomainInformationCustomSave(TestCase): class TestDomainRequestIncomplete(TestCase): def setUp(self): super().setUp() + self.factory = RequestFactory() username = "test_user" first_name = "First" last_name = "Last" @@ -2012,7 +2015,10 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.is_policy_acknowledged = None self.assertFalse(self.domain_request._is_policy_acknowledgement_complete()) - def test_form_complete(self, request): + def test_form_complete(self): + request = self.factory.get("/") + request.user = self.user + self.assertTrue(self.domain_request._form_complete(request)) self.domain_request.generic_org_type = None self.domain_request.save() From b8bc2900cce9c04dfe48de399411d440f1331e25 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 24 Jun 2024 14:24:33 -0700 Subject: [PATCH 20/21] Add comment --- src/registrar/models/domain_request.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 49b84d472..63f11c8d0 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1174,6 +1174,7 @@ class DomainRequest(TimeStampedModel): and self._is_authorizing_official_complete() and self._is_requested_domain_complete() and self._is_purpose_complete() + # NOTE: This flag leaves submitter as empty (request wont submt) hence preset to True and (self._is_submitter_complete() if not has_profile_feature_flag else True) and self._is_other_contacts_complete() and self._is_additional_details_complete() From 4e3b7a49bc02c6ca0d1b763038a629f4fe2cbd1a Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 24 Jun 2024 14:25:21 -0700 Subject: [PATCH 21/21] Spelling error fix --- src/registrar/models/domain_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 63f11c8d0..25b696c2a 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1174,7 +1174,7 @@ class DomainRequest(TimeStampedModel): and self._is_authorizing_official_complete() and self._is_requested_domain_complete() and self._is_purpose_complete() - # NOTE: This flag leaves submitter as empty (request wont submt) hence preset to True + # NOTE: This flag leaves submitter as empty (request wont submit) hence preset to True and (self._is_submitter_complete() if not has_profile_feature_flag else True) and self._is_other_contacts_complete() and self._is_additional_details_complete()