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

fix: select fields on request pages (#4069 partial) #4099

Merged
merged 9 commits into from
Feb 25, 2024

Conversation

elasticspoon
Copy link
Collaborator

Description

Issue #4069 has two parts:

  1. Make a bad request, go to dashboard, make a good request => request has strange values
  2. Make a bad request => inputs field on page break

This PR (so far) fixes part 2:

After a request bad request is submitted the select input disappears and turns into a text input:
This was caused by formatted_requestable_items not being set. PR set value.

Additionally, the select field has different values on a new request vs failed request:
Refactors setting @formatted_requestable_items to a private method call. Unifies all the behavior of setting requestable items.

Part 1 is not addressed. I have been unable to replicate locally.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Added systems tests to test the select inputs.

Addresses part of issue rubyforgood#4069:

After a request bad request is submitted the select
input disappears and turns into a text input.

This was caused by formatted_requestable_items not being set.

Refactors setting @formatted_requestable_items to a private method call.
Unifies all the behavior of setting requestable items.

Adds additional tests to ensure select fields have
correct values.

feat: adds TODOs
@elasticspoon elasticspoon marked this pull request as ready for review February 12, 2024 17:54
@elasticspoon elasticspoon changed the title fix: select fields on request pages (#4069 partial) WIP fix: select fields on request pages (#4069 partial) Feb 12, 2024
Renames formatted_requestable_items to requestable_items

Moves formatting and sorting to service object
Adds tests for service object

Removes method with side effect in favor of direct call to
service object to set @requestable_items

refactor: removes unneeded code
Removes testing of service object from system spec
Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

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

Looking good! Had one request.

Conflicts:
    app/views/partners/requests/new.html.erb
Previously tests were simply re-stating the original code.

Now specific items are set on the org and partner
The tests then check in the service object return
a correct subset of those items.
Tried to make it a bit more clear what is getting set up in
the service.
fix: set name manually

fix: change bad request test
Adds a test to make sure incorrect inputs do not
get cleared on submission.
app/views/partners/requests/new.html.erb Outdated Show resolved Hide resolved
spec/system/partners/managing_requests_system_spec.rb Outdated Show resolved Hide resolved
@elasticspoon
Copy link
Collaborator Author

@dorner i have removed unrelated tests and opened #4135 and #4134 with the issues I found when doing this PR.

Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification - this looks good!

@awwaiid awwaiid merged commit 98e2f1d into rubyforgood:main Feb 25, 2024
19 checks passed
@elasticspoon elasticspoon deleted the 4069-individual-requests-issue branch February 25, 2024 16:03
@cielf
Copy link
Collaborator

cielf commented Mar 2, 2024

@elasticspoon , @dorner The field is a drop-down again! Yay!

Alas, on manually testing this on staging before we push to production, I found that the content is still disappearing.

This is definitely a step in the right direction, though, so I'm going to recommend we push it to production.

@elasticspoon
Copy link
Collaborator Author

elasticspoon commented Mar 2, 2024

That fix got moved to another PR #4142 that has not yet been merged. Unless additional content stopped getting preserved? It should only be values on individual request pages that don't get preserved.

@cielf
Copy link
Collaborator

cielf commented Mar 2, 2024

I've only seen the values (at least all the items, whether or not complete) on the individual request pages not getting preserved. So, ok - it's on another PR.

Copy link
Contributor

github-actions bot commented Mar 3, 2024

@elasticspoon: Your PR fix: select fields on request pages (#4069 partial) is part of today's Human Essentials production release: 2024.03.03.
Thank you very much for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants