This repository was archived by the owner on Nov 6, 2021. It is now read-only.
Customizable partner form#287
Merged
seanmarcia merged 5 commits intomasterfrom Apr 12, 2020
Merged
Conversation
3 tasks
edwinthinks
reviewed
Apr 6, 2020
Collaborator
edwinthinks
left a comment
edwinthinks
left a comment
There was a problem hiding this comment.
Looks pretty good! I left some comments :)
| end | ||
|
|
||
| def valid_partner_form_creation_request( | ||
| diaper_bank_id: 123, |
Collaborator
There was a problem hiding this comment.
Suggested change
| diaper_bank_id: 123, | |
| diaper_bank_id: Faker::Number.number, |
|
|
||
| describe "#partials_to_show" do | ||
| let(:partner) { create(:partner, diaper_bank_id: 100) } | ||
| it 'has 9 partials when there are no displayable partials configured' do |
Collaborator
There was a problem hiding this comment.
Suggested change
| it 'has 9 partials when there are no displayable partials configured' do | |
| it 'has 9 partials when there are no displayable partials configured' do |
|
|
||
| it 'displays the number of displayable partials when they are configured' do | ||
| partner.diaper_bank_id = 100 | ||
| FactoryBot.create(:partner_form, diaper_bank_id: 100, |
Collaborator
There was a problem hiding this comment.
Suggested change
| FactoryBot.create(:partner_form, diaper_bank_id: 100, | |
| FactoryBot.create(:partner_form, diaper_bank_id: partner.id, |
Since partner is already defined above with let could we just use the partner from there?
| describe "#partials_to_show" do | ||
| let(:partner) { create(:partner, diaper_bank_id: 100) } | ||
| it 'has 9 partials when there are no displayable partials configured' do | ||
| expect(partner.partials_to_show.size).to eq(9) |
Collaborator
There was a problem hiding this comment.
Suggested change
| expect(partner.partials_to_show.size).to eq(9) | |
| expect(partner.partials_to_show).to eq(Partner::ALL_PARTIALS) |
^ this might make the test a bit more robust, so when you add a new field you won't have to worry about fixing this spec.
| end | ||
|
|
||
| def displayable_partials | ||
| PartnerForm.find_by(diaper_bank_id: diaper_bank_id)&.sections |
Collaborator
There was a problem hiding this comment.
Suggested change
| PartnerForm.find_by(diaper_bank_id: diaper_bank_id)&.sections | |
| PartnerForm.find_by(diaper_bank_id: diaper_bank_id)&.sections |
Can we can add the has_one association to get this instead of putting a query here?
has_one :partner_form, primary_key: :diaper_bank_id, foreign_key: :diaper_bank_id
^ This should work and allow you to use partner_form in place of the Partner.find_by(...)
| displayable_partials || ALL_PARTIALS | ||
| end | ||
|
|
||
| def displayable_partials |
Collaborator
There was a problem hiding this comment.
Could we put this as a private method since it is only being used in the partial_to_show right now?
|
|
||
| RSpec.describe PartnerForm, type: :model do | ||
| subject do | ||
| described_class.new(diaper_bank_id: 1) |
Collaborator
There was a problem hiding this comment.
Suggested change
| described_class.new(diaper_bank_id: 1) | |
| described_class.new(diaper_bank_id: Faker::Number.number) |
seanmarcia
force-pushed
the
customizable_partner_form
branch
from
0d9ed76 to
c48e318
Compare
seanmarcia
force-pushed
the
customizable_partner_form
branch
from
c48e318 to
dcdad79
Compare
Member
Author
|
Addressed your comments. Most of them anyway. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is part of the customizations to allow diaper banks to select which sections of the partner form they want them to be able to edit.