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

Separate SOW & PAF form editing #4261

Merged
merged 10 commits into from
Dec 17, 2024
Merged

Separate SOW & PAF form editing #4261

merged 10 commits into from
Dec 17, 2024

Conversation

wes-otf
Copy link
Contributor

@wes-otf wes-otf commented Dec 6, 2024

Fixes #4204. Moves the SOW out into it's own form when editing. This also removes the user_has_updated_details attribute in favor of using a property for both the project form & SOW that checks the field_data on the respective form. This allows for tracking of the project form & SOW independently.

Also a few small aesthetic changes bundled in, like margin additions & hiding of submission attachments sidebar when there's none.

Screenshots

appearance when first creating a project
Screenshot 2024-12-06 at 16 07 24

appearance after editing both forms
Screenshot 2024-12-06 at 16 04 29

Test Steps

  • When viewing the details of a project with both a Project Form & SOW, ensure that both appear under Project documents.
  • Confirm that both can be edited, saved, and viewed on their own
  • Confirm that when creating a project from a fund that has a Project Form & SOW, a todo list item is created for both

@wes-otf wes-otf added the Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). label Dec 6, 2024
@wes-otf wes-otf requested review from frjo and sandeepsajan0 December 6, 2024 21:08
Copy link
Member

@frjo frjo left a comment

Choose a reason for hiding this comment

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

This is a good change, it was a bit of a hack having them merged in the first place.

Found some minor issues.

How will the SOW now fit in to the approval workflow?

@@ -168,6 +167,7 @@
&__actions {
display: flex;
align-items: center;
margin-left: auto;
Copy link
Member

Choose a reason for hiding this comment

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

Please use margin-inline-start instead.

Copy link
Member

Choose a reason for hiding this comment

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

ping

@@ -44,12 +44,13 @@
PAF_REQUIRED_CHANGES,
PAF_WAITING_APPROVAL,
PAF_WAITING_ASSIGNEE,
PROJECT_SUBMIT_PAF,
PROJECT_SUBMIT_PF,
Copy link
Member

Choose a reason for hiding this comment

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

As a machine name we use "PAF" everywhere so I think it is best to stick with that.

I do not mind changing it to "PF" or "PROJECT_FORM" etc. but it should be done everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

Ping

<div class="card card--solid">
{% if object.sow.output_answers %}
<div class="rich-text rich-text--answers">
<div class="simplified__paf_answers">
Copy link
Member

Choose a reason for hiding this comment

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

This class was removed in #4196.

Copy link
Member

Choose a reason for hiding this comment

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

Are not the "rich-text rich-text--answers" ok to use? We use them in most other places when we render streamfields.

@wes-otf
Copy link
Contributor Author

wes-otf commented Dec 11, 2024

all that left is getting these tests passing - a bit of a battle because my flag uses the project's form_data, but that won't get written unless a valid FormFieldBlock wrapped in a StreamValue is passed in the save method kwargs. it's giving me a hard time when I try to mock it

def save(self, *args, **kwargs):
self.instance.form_fields = kwargs.pop("paf_form_fields", {})
self.instance.form_data = {
field: self.cleaned_data[field]
for field in self.instance.question_field_ids
if field in self.cleaned_data
}

@frjo
Copy link
Member

frjo commented Dec 11, 2024

@wes-otf How vital is the test? We could remove it.

@wes-otf
Copy link
Contributor Author

wes-otf commented Dec 11, 2024

@frjo Yeah that sounds good - I don't think it's vital as it'd really just be checking if form_data is an empty dict. I tossed it and added your suggestions

@frjo frjo added the Type: Minor Minor change, used in release drafter label Dec 12, 2024
@wes-otf
Copy link
Contributor Author

wes-otf commented Dec 12, 2024

grabbed some final bugs but should be ready to be deployed to test now @frjo

@frjo frjo added Status: Needs testing Tickets that need testing/qa Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Dec 13, 2024
@frjo frjo force-pushed the enhancement/seperate-paf-and-sow branch from 36f699c to a80262c Compare December 13, 2024 20:50
@frjo
Copy link
Member

frjo commented Dec 16, 2024

@wes-otf When I download a SOW at http://apply.hypha.test:9001/apply/projects/4/sow/ no fields are added to the exports. I guess something changes regarding getting the fields?

When I test the main branch locally it seems to work as it should.

@wes-otf wes-otf force-pushed the enhancement/seperate-paf-and-sow branch from 12ea3d5 to b218be1 Compare December 16, 2024 21:54
@wes-otf
Copy link
Contributor Author

wes-otf commented Dec 16, 2024

@frjo great catch! my logic was copy & pasted, checking self rather than project - also removed some mentions of the SOW in the PAF export.

@frjo
Copy link
Member

frjo commented Dec 17, 2024

Deploying the latest to test now.

@frjo
Copy link
Member

frjo commented Dec 17, 2024

Can confirm that the bug in sow exports are now fixed. Looks all good to me.

@wes-otf
Copy link
Contributor Author

wes-otf commented Dec 17, 2024

@frjo thanks for testing - I think it's ready to go then! I did more testing on my end too and didn't see anything

@frjo frjo added Status: Tested - approved for live ✅ and removed Status: Needs testing Tickets that need testing/qa Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Dec 17, 2024
@frjo frjo merged commit 6ca172a into main Dec 17, 2024
12 checks passed
@frjo frjo deleted the enhancement/seperate-paf-and-sow branch March 11, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Tested - approved for live ✅ Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Minor Minor change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project Form/SOW workflow can be confusing
2 participants