-
-
Notifications
You must be signed in to change notification settings - Fork 504
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
Superadmin Partners View (4682) & Category Dropdown Error (4674) #4703
base: main
Are you sure you want to change the base?
Changes from all commits
a3fa8d6
c1aeb5c
ab06f12
12bed2e
8583e34
4fd0140
186fc3b
a67431a
a65d019
20f4ba5
94d4728
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
RSpec.describe Admin::PartnersController, type: :controller do | ||
let(:organization) { create(:organization) } | ||
|
||
# Use the super_admin instead of organization_admin | ||
let(:super_admin) { create(:super_admin) } | ||
|
||
# Ensure partners are created before the test | ||
let!(:partner2) { create(:partner, name: "Bravo", organization: organization) } | ||
let!(:partner1) { create(:partner, name: "alpha", organization: organization) } | ||
let!(:partner3) { create(:partner, name: "Zeus", organization: organization) } | ||
|
||
let(:default_params) do | ||
{organization_id: organization.id} | ||
end | ||
|
||
context "When logged in as a super admin" do | ||
before do | ||
sign_in(super_admin) # Sign in as the super_admin | ||
end | ||
|
||
describe "GET #index" do | ||
it "returns http success" do | ||
get :index | ||
expect(response).to be_successful | ||
end | ||
|
||
it "assigns partners ordered by name (case-insensitive)" do | ||
get :index | ||
expect(assigns(:partners)).to eq([partner1, partner2, partner3].sort_by { |p| p.name.downcase }) | ||
end | ||
end | ||
end | ||
|
||
context "When logged in as a non-admin user" do | ||
let(:user) { create(:user) } | ||
|
||
before do | ||
sign_in(user) | ||
end | ||
|
||
describe "GET #index" do | ||
it "redirects to login or unauthorized page" do | ||
get :index | ||
expect(response).to be_redirect | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -111,6 +111,7 @@ | |
|
||
describe "CREATE #create" do | ||
let!(:existing_item) { create(:item, organization: organization, name: "Really Good Item") } | ||
let!(:item_category) { create(:item_category, organization: organization, name: "Test Category") } | ||
|
||
describe "with an already existing item name" do | ||
let(:item_params) do | ||
|
@@ -134,6 +135,49 @@ | |
end | ||
end | ||
|
||
describe "with invalid parameters" do | ||
let(:invalid_item_params) do | ||
{ | ||
item: { | ||
name: "", # Invalid name | ||
partner_key: create(:base_item).partner_key, | ||
value_in_cents: -100, # Invalid value | ||
package_size: nil, | ||
distribution_quantity: nil | ||
} | ||
} | ||
end | ||
|
||
let(:categories) do | ||
[ | ||
FactoryBot.create(:item_category, name: 'Apples', organization: organization), | ||
FactoryBot.create(:item_category, name: 'Umbrella', organization: organization), | ||
FactoryBot.create(:item_category, name: 'Bananas', organization: organization) | ||
] | ||
end | ||
|
||
before do | ||
categories # Ensure categories are created before the request | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can remove this |
||
|
||
it "loads and displays the item categories when rendering new" do | ||
|
||
# Attempt to create an item with invalid parameters | ||
post items_path, params: invalid_item_params | ||
|
||
# Expect to render the new template | ||
expect(response).to render_template(:new) | ||
|
||
# Ensure the item categories are assigned in the controller | ||
expect(assigns(:item_categories)).to eq(organization.item_categories.order('name ASC')) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I much prefer giving the categories explicit values and testing against those values rather than pulling them from the DB. You end up basically just copying the code you're testing into the test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I'm having trouble understanding exactly what you mean here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean doing something like let(:categories) do
[
FactoryBot.create(:item_category, name: 'Apples'), # include whatever other IDs need to be passed in
FactoryBot.create(:item_category, name: 'Bananas')
]
end
# ...
expect(assigns(:item_categories)).to eq(categories)) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @bdeanhardt -- do you need further guidance on this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @cielf! Past 2 weeks have been chaos, but I'm back! I'm taking a look today :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So sorry about the delay! How does it look now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You shouldn't need to specify an To be a bit more directive, here's what I had in mind: let!(:category1) { FactoryBot.create(:item_category, name: 'Bananas', organization: organization) }
let!(:category2) { FactoryBot.create(:item_category, name: 'Apples' organization: organization) }
let!(:category3) { FactoryBot.create(:item_category, name: 'Umbrella', organization: organization) }
# ...
expect(assigns(:item_categories)).to eq([category2, category1, category3]) |
||
|
||
# Verify the categories are included in the response body | ||
categories.each do |category| | ||
expect(response.body).to include("<option value=\"#{category.id}\">#{category.name}</option>") | ||
end | ||
end | ||
end | ||
|
||
describe 'GET #index' do | ||
let(:storage_location) { create(:storage_location, organization: organization) } | ||
let!(:item) { create(:item, organization: organization, name: "ACTIVEITEM") } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't need to sort since you know the partners' names. :) Just reorder the array to
partner2, partner1, partner3
.Actually... can you switch it so the
partner1
line is moved above thepartner2
line? That way they'd actually be created out of order. The order of thelet!
statements is what matters.