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

Community resource addresses #900

Conversation

leahriffell
Copy link
Contributor

@leahriffell leahriffell commented Mar 10, 2021

Why

closes #879

  • Previously: when user creates new Community Resource, they must select from dropdown of addresses of existing Locations
  • Now: when user creates new Community Resource, they input an address
    • If Location with this address and type already exists, associates new Community Resource to this Location
    • If Location with this address and type does not exist, creates new Location and associates new Community Resource to this Location

What

Previously:
Screen Shot 2021-03-09 at 7 56 46 PM

Now:
Screen Shot 2021-03-09 at 7 53 54 PM

How

  • Used first_or_create to find or create a new Location

Testing

  • Updated existing request spec to add address params

Next Steps

  • Refactor into form object design pattern + add unit tests
  • Want to add more testing - ex: new location vs. location already existing

Outstanding Questions, Concerns and Other Notes

  • Do we want to collect any other Location data on this form? (ex: county, region, neighborhood)
  • I couldn't figure out how to get label to display on Location Type form field (even when adding, label "Location Type"). I think this may have something to do with Simple Form?
  • Do we want address to be required field on new Community Resource form?
    • aka do we want to require that a new Community Resource belongs to a Location?

Accessibility

N/A

Security

This issue may need to be reviewed with accessibility and security in mind as I'm not very familiar with how I've impacted them

Pre-Merge Checklist

  • Security & accessibility have been considered
  • Tests have been added, or an explanation has been given why the features cannot be tested
  • Documentation and comments have been added to the codebase where required
  • Entry added to CHANGELOG.md if appropriate
  • Outstanding questions and concerns have been resolved
  • Any next steps have been turned into Issues or Discussions as appropriate

).first_or_create

community_resource.location = location
community_resource.update permitted_attributes(community_resource)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Want to refactor this using the form design pattern but wanted to at least share what has been done in case someone else has time to get to this before I can.

<%= location_form.input :city, required: true %>
<%= location_form.input :state, required: true, maxlength: 2 %>
<%= location_form.input :zip, required: true, maxlength: 5 %>
<%= location_form.collection_select :location_type_id, LocationType.order(:name), :id, :name, include_blank: false, required: true %>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the form field that I cannot figure out how to get a label to display on. I think this may have something to do with Simple Form because even when adding label: "xyz"I don't have any luck.

@@ -17,7 +17,18 @@ def new
end

def create
community_resource.assign_attributes permitted_attributes(community_resource)
location_params = params['community_resource']['location']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If Location is not required on a new Community Resource, I will need to add some checks in place here (currently required form fields).

@solebared solebared changed the base branch from main to fix-spec-on-pr-900 March 19, 2021 01:33
@solebared solebared merged commit 7fdd819 into rubyforgood:fix-spec-on-pr-900 Mar 19, 2021
This was referenced Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants