Apetty/chal 1742 allow cm to set required submission uploads v4 (#1220)

* Add new fields to Challenge schema via migrations

Include submission_collection_method in challenges to specify how submissions are collected.

Add file_upload_required boolean field to determine if file uploads are necessary.

Insert upload_instruction_note field to provide file upload guidance to users.

* Enhance Content Manager experience for submission links

- Allow Content Managers to require file uploads for internal submissions.

- Enable Content Managers to add instructions for Solvers on file uploads.

- Ensure the how-to-enter flow correctly handles internal submissions by setting 'how_to_enter_link' to nil.

- Update how-to-enter flow for external submission types to clear 'upload_instruction_note' and 'file_upload_required' to avoid saving outdated information.

Note: Solver interface implementation for this feature is pending at the time of this commit.

* Implement file upload requirement validation for submissions

- Enabled successful submission postings without a required file upload ().

- Introduced server-side validation for mandatory file uploads (), failing submissions without attached files with .

- Verified server behavior for successful submissions with required file uploads when a document is attached, indicated by  responses in logs.

- Implemented conditional validation logic, now accurately recognizes and enforces the  flag:

- Established robust logging to confirm functionality of the upload requirement feature.

- Ensured  remains correctly associated with  during file attachments, for both  flag states.

- Note: Care taken not to disrupt existing submission-document association process.

* Refactor submission handling and form validation

- Fixed compiler warnings related to undefined or unused variables.

- Adjusted the solver submission form to conditionally enforce file uploads based on challenge manager configurations.

- Applied intended style updates to enhance the form's user interface.

* Fix submission process issues

- Add missing assigns to render calls in submission_controller
- Refactor update action to use helper functions for clarity
- Correct naming conflict with render function
- Ensure file upload validation is enforced during submission

* Format code with mix format

* Refactor code based on credo feedback

* Apply Elixir formatter to submissions.ex

* Refactor submissions tests and update migrations to handle file uploads

- Updated migrations to ensure required columns only added if not exist

- Adjusted submissions tests to pass correct arity to function calls

- Refactored unit test code to account for new file upload requirements in challenges

* minor tuning

* Fix incorrect arity in Submissions.update_review calls within tests

* Fix syntax error in migration file

* Worked around Ecto schema preload issue in submission tests

.tool-versions

@@ -1,5 +1,5 @@
 elixir 1.12.3
 erlang 23.3.2
-#nodejs 16.13.0
-nodejs 18.17.1
-yarn 1.22.5
+#erlang 24.2
+nodejs 16.13.0
+yarn 1.22.5

lib/challenge_gov/challenges/challenge.ex

@@ -122,6 +122,10 @@ defmodule ChallengeGov.Challenges.Challenge do
     field(:is_multi_phase, :boolean)
     field(:terms_equal_rules, :boolean)
 
+    field(:file_upload_required, :boolean)
+    field(:upload_instruction_note, :string)
+    field(:submission_collection_method, :string)
+
     # Virtual Fields
     field(:logo, :string, virtual: true)
 
@@ -326,7 +330,10 @@ defmodule ChallengeGov.Challenges.Challenge do
       :how_to_enter_link,
       :announcement,
       :announcement_datetime,
-      :short_url
+      :short_url,
+      :file_upload_required,
+      :upload_instruction_note,
+      :submission_collection_method
     ])
     |> cast_assoc(:non_federal_partners, with: &NonFederalPartner.draft_changeset/2)
     |> cast_assoc(:events)
@@ -555,11 +562,41 @@ defmodule ChallengeGov.Challenges.Challenge do
     |> cast_assoc(:phases, with: &Phase.judging_changeset/2)
   end
 
-  def how_to_enter_changeset(struct, _params) do
+  def how_to_enter_changeset(struct, params) do
+    method = Map.get(params, "submission_collection_method")
+
+    modified_params =
+      case method do
+        "internal" ->
+          # When the method is internal, clear the how_to_enter_link
+          # as it isn't relevant for internal submissions
+          Map.put(params, "how_to_enter_link", nil)
+
+        "external" ->
+          # When the method is external, clear the fields that are only relevant for internal method
+          params
+          |> Map.put("upload_instruction_note", nil)
+          |> Map.put("file_upload_required", nil)
+
+        _ ->
+          # If there's no specific method or an unrecognized one,
+          # don't modify the incoming params
+          params
+      end
+
+    # Now cast the parameters. Whether modified or not, they need to match
+    # the fields expected for the struct.
     struct
+    |> cast(modified_params, ~w(how_to_enter_link upload_instruction_note file_upload_required)a)
+    # Also associate the phases with the struct
     |> cast_assoc(:phases, with: &Phase.how_to_enter_changeset/2)
   end
 
+  # def how_to_enter_changeset(struct, _params) do
+  #   struct
+  #   |> cast_assoc(:phases, with: &Phase.how_to_enter_changeset/2)
+  # end
+
   def resources_changeset(struct, _params) do
     struct
     |> force_change(:faq, fetch_field!(struct, :faq))

lib/challenge_gov/submissions.ex

@@ -9,7 +9,7 @@ defmodule ChallengeGov.Submissions do
 
   alias ChallengeGov.Accounts
   alias ChallengeGov.Emails
-  alias ChallengeGov.GovDelivery
+  # alias ChallengeGov.GovDelivery
   alias ChallengeGov.Mailer
   alias ChallengeGov.Repo
   alias ChallengeGov.SecurityLogs
@@ -100,6 +100,7 @@ defmodule ChallengeGov.Submissions do
   def create_draft(params, user, challenge, phase) do
     params = attach_default_multi_params(params)
     changeset = Submission.draft_changeset(%Submission{}, params, user, challenge, phase)
+    # |> validate_file_upload(challenge, params)
 
     Ecto.Multi.new()
     |> Ecto.Multi.insert(:submission, changeset)
@@ -125,40 +126,105 @@ defmodule ChallengeGov.Submissions do
     end
   end
 
-  def create_review(params, user, challenge, phase) do
-    params = attach_default_multi_params(params)
-    changeset = Submission.review_changeset(%Submission{}, params, user, challenge, phase)
+  # def create_review(params, user, challenge, phase) do
+  #   params = attach_default_multi_params(params)
 
-    Ecto.Multi.new()
-    |> Ecto.Multi.insert(:submission, changeset)
-    |> attach_documents(params)
-    |> Repo.transaction()
-    |> case do
+  #   # Create the changeset independently
+  #   changeset = 
+  #     Submission.review_changeset(%Submission{}, params, user, challenge, phase)
+  #     |> validate_file_upload(challenge, params) # start pipe here
+
+  #   # Initialize the Multi and include the changeset
+  #   Ecto.Multi.new()
+  #   |> Ecto.Multi.insert(:submission, changeset)
+  #   |> attach_documents(params)
+  #   |> Repo.transaction()
+  #   |> case do
+  #     {:ok, %{submission: submission}} ->
+  #       submission = new_form_preload(submission)
+  #       if submission.manager_id, do: send_submission_review_email(user, phase, submission)
+
+  #       {:ok, submission}
+
+  #     {:error, _type, changeset, _changes} ->
+  #       changeset = preserve_document_ids_on_error(changeset, params)
+
+  #       changeset = %Ecto.Changeset{
+  #         changeset
+  #         | data: Repo.preload(changeset.data, [:documents, :submitter])
+  #       }
+
+  #       {:error, changeset}
+  #   end
+  # end
+
+  def create_review(params, user, challenge, phase) do
+    params_with_defaults = attach_default_multi_params(params)
+
+    # Initialize the submission and pipe it through the changeset function and validation
+    changeset =
+      %Submission{}
+      |> Submission.review_changeset(params_with_defaults, user, challenge, phase)
+      |> validate_file_upload(challenge, params_with_defaults)
+
+    # Initialize the Multi and include the changeset
+    multi =
+      Ecto.Multi.new()
+      |> Ecto.Multi.insert(:submission, changeset)
+      |> attach_documents(params_with_defaults)
+      |> Repo.transaction()
+
+    # Handle the transaction result
+    case multi do
       {:ok, %{submission: submission}} ->
         submission = new_form_preload(submission)
         if submission.manager_id, do: send_submission_review_email(user, phase, submission)
-
         {:ok, submission}
 
       {:error, _type, changeset, _changes} ->
-        changeset = preserve_document_ids_on_error(changeset, params)
+        changeset
+        |> preserve_document_ids_on_error(params_with_defaults)
+        |> Repo.preload([:documents, :submitter])
+        # Ensuring the return is a tuple
+        |> (fn cs -> {:error, cs} end).()
+    end
+  end
 
-        changeset = %Ecto.Changeset{
-          changeset
-          | data: Repo.preload(changeset.data, [:documents, :submitter])
-        }
+  defp validate_file_upload(changeset, challenge, params) do
+    if challenge.file_upload_required do
+      is_documents_loaded = changeset.data.documents != Ecto.Association.NotLoaded
 
-        {:error, changeset}
+      document_ids =
+        if changeset.data.id && is_documents_loaded do
+          Enum.map(changeset.data.documents, & &1.id)
+        else
+          # Default to empty if it's a new record or documents are not loaded
+          []
+        end
+
+      case params["document_ids"] || document_ids do
+        [] ->
+          Ecto.Changeset.add_error(
+            changeset,
+            :document_ids,
+            "At least one file must be attached."
+          )
+
+        _ ->
+          changeset
+      end
+    else
+      changeset
     end
   end
 
   def edit(submission) do
     Submission.changeset(submission, %{})
   end
 
-  def update_draft(submission, params) do
+  def update_draft(submission, params, challenge) do
     params = attach_default_multi_params(params)
-    changeset = Submission.update_draft_changeset(submission, params)
+    changeset = Submission.update_draft_changeset(submission, params, challenge)
 
     Ecto.Multi.new()
     |> Ecto.Multi.update(:submission, changeset)
@@ -174,9 +240,9 @@ defmodule ChallengeGov.Submissions do
     end
   end
 
-  def update_review(submission, params) do
+  def update_review(submission, params, challenge) do
     params = attach_default_multi_params(params)
-    changeset = Submission.update_review_changeset(submission, params)
+    changeset = Submission.update_review_changeset(submission, params, challenge)
 
     Ecto.Multi.new()
     |> Ecto.Multi.update(:submission, changeset)

lib/challenge_gov/submissions/submission.ex

@@ -121,14 +121,12 @@ defmodule ChallengeGov.Submissions.Submission do
     |> validate_inclusion(:status, status_ids())
     |> validate_review_verify(params)
     |> validate_terms(params)
-    |> validate_required([
-      :title,
-      :brief_description,
-      :description
-    ])
+    |> validate_required([:title, :brief_description, :description])
+    # Validate file upload on review.
+    |> validate_file_upload(challenge, params)
   end
 
-  def update_draft_changeset(struct, params) do
+  def update_draft_changeset(struct, params, challenge) do
     struct
     |> changeset(params)
     |> put_change(:status, "draft")
@@ -139,8 +137,7 @@ defmodule ChallengeGov.Submissions.Submission do
     |> validate_inclusion(:status, status_ids())
   end
 
-  # the review chance to submited
-  def update_review_changeset(struct, params) do
+  def update_review_changeset(struct, params, challenge) do
     struct
     |> changeset(params)
     |> put_change(:status, "submitted")
@@ -151,11 +148,27 @@ defmodule ChallengeGov.Submissions.Submission do
     |> validate_inclusion(:status, status_ids())
     |> validate_review_verify(params)
     |> validate_terms(params)
-    |> validate_required([
-      :title,
-      :brief_description,
-      :description
-    ])
+    |> validate_required([:title, :brief_description, :description])
+    # Validate file upload on update review.    
+    |> validate_file_upload(challenge, params)
+  end
+
+  defp validate_file_upload(changeset, challenge, params) do
+    if challenge.file_upload_required do
+      case params["document_ids"] || changeset.data.documents do
+        [] ->
+          add_error(
+            changeset,
+            :document_ids,
+            "At least one file must be uploaded with your submission."
+          )
+
+        _ ->
+          changeset
+      end
+    else
+      changeset
+    end
   end
 
   def submit_changeset(struct) do