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

Make dropdown lists alphabetical order ( Part 1) #4663

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -801,4 +801,4 @@ DEPENDENCIES
webmock (~> 3.23)

BUNDLED WITH
2.5.16
2.5.19
2 changes: 1 addition & 1 deletion app/controllers/adjustments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def index
@paginated_adjustments = @adjustments.page(params[:page])

@storage_locations = Adjustment.storage_locations_adjusted_for(current_organization).uniq
@users = current_organization.users
@users = current_organization.users.sort_by { |user| user.name.to_s.downcase }

respond_to do |format|
format.html
Expand Down
2 changes: 1 addition & 1 deletion app/models/adjustment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Adjustment < ApplicationRecord
validate :storage_locations_belong_to_organization

def self.storage_locations_adjusted_for(organization)
includes(:storage_location).joins(:storage_location).where(organization_id: organization.id, storage_location: {discarded_at: nil}).collect(&:storage_location).sort
includes(:storage_location).joins(:storage_location).where(organization_id: organization.id, storage_location: {discarded_at: nil}).collect(&:storage_location)
end

def split_difference
Expand Down
2 changes: 1 addition & 1 deletion app/models/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class Item < ApplicationRecord
scope :loose, -> { where(kit_id: nil) }

scope :visible, -> { where(visible_to_partners: true) }
scope :alphabetized, -> { order(:name) }
scope :alphabetized, -> { order('LOWER(name)') }
scope :by_base_item, ->(base_item) { where(base_item: base_item) }
scope :by_partner_key, ->(partner_key) { where(partner_key: partner_key) }

Expand Down
1 change: 0 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ class User < ApplicationRecord
validate :password_complexity

default_scope -> { kept }
scope :alphabetized, -> { order(discarded_at: :desc, name: :asc) }
Copy link
Author

Choose a reason for hiding this comment

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

This wasn't being used anywhere, so I opted to remove it and look to order user select fields on the instance vars defined in relevant controller files.

scope :partner_users, -> { with_role(Role::PARTNER, :any) }
scope :org_users, -> { with_role(Role::ORG_USER, :any) }
scope :search_name, ->(query) { where("name ilike ?", "%#{query}%") }
Expand Down
5 changes: 3 additions & 2 deletions app/views/line_items/_line_item_fields.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
<span class="li-name w-100">
<%= field.input :item_id,
disabled: requested.present?,
collection: @items, prompt: "Choose an item",
include_blank: "",
collection: @items,
prompt: "Choose an item",
include_blank: false, # We've got a prompt, so no need for an include blank
Copy link
Author

Choose a reason for hiding this comment

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

I can remove this comment later, but I observed the item choice dropdown was showing some weird UI behavior, screenshot below of what I was seeing

image

Copy link
Author

@ajistrying ajistrying Sep 22, 2024

Choose a reason for hiding this comment

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

With the change, here is what it looks like. I think it looks a lot better, especially since this isn't a strict dropdown:
image

label: false,
input_html: { class: "my-0 line_item_name", "data-controller": "select2" } %>
<% if requested.present? %>
Expand Down
3 changes: 2 additions & 1 deletion app/views/storage_locations/_source.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
label: label,
error: error,
selected: storage_location_for_source(source.object),
include_blank: true,
include_blank: false,
prompt: "Choose a storage location",
Copy link
Author

Choose a reason for hiding this comment

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

Felt like this just looked better.

Before:
image

After:
image

input_html: {
data: {
storage_location_inventory_path: inventory_storage_location_path(
Expand Down
4 changes: 3 additions & 1 deletion db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,9 @@ def random_record_for_org(org, klass)
user = User.create(
email: user_data[:email],
password: 'password!',
password_confirmation: 'password!'
password_confirmation: 'password!',
organization: user_data[:organization], # Any reason not to set this?
name: user_data[:email].split('@').first # Any reason not to set this?
Copy link
Author

Choose a reason for hiding this comment

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

Was there a reason why we weren't ensuring that an organization and name were in place for users?

Copy link
Collaborator

@cielf cielf Sep 23, 2024

Choose a reason for hiding this comment

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

Once upon a time we needed organizations for bank users. We changed to a role-based access for all users so it is no longer needed. Super users and partners wouldn't have had an organization from the get-go. As for names, I think all the bank users in production have names, but not all the partner users do. Does that help?

Copy link
Author

Choose a reason for hiding this comment

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

Ahhhh I see, that does help thank you! I'll get rid of these two additions and keep that in mind moving forward.

)

if user_data[:organization]
Expand Down