Daniel Orner comments

Results 405 comments of


                                            Daniel Orner

Add missing kit rspecs, DRY up kit base items and report service

@jimmyli97 are you able to finish this off?

Add missing kit rspecs, DRY up kit base items and report service

All good on my side. @cielf did you want to kick the tires?

Fix behaviour with errors on Distribution

I think rescuing those errors per event in `handle_inventory_event` and collating them together might be fine. Ideally you'd keep track of the first error and just keep appending to its...

Fix behaviour with errors on Distribution

I'd rather change `InventoryError` to be the aggregate and rename the immediate single error (e.g. `InventoryActionError`). Only one place looks at the `item` on InventoryError and does anything with it....

Should be able to select items for audit that have 0 inventory.

@danielabar just to confirm - `inventory_items` themselves no longer exist at all. We've replaced them with inventory calculated via events.

Add [DO NOT REPLY] to the name for the accounts email.

We definitely don't need to override the actual account. Just the `from`. And yes, ideally it should match everywhere. @cielf up to you if you prefer the `DO NOT REPLY`...

Product totals - custom units

I'd probably prefer it like this: * Pads (individual) * Pads (in packs)

Need a 'graceful landing' if someone goes to a screen they don't have access to while logged in.

@therufs not sure I understand what's changed in that PR. As far as I can tell you renamed a method, but I don't really see how that fixes this problem?

Need a 'graceful landing' if someone goes to a screen they don't have access to while logged in.

I'm still a bit confused. ApplicationController has this as a filter: ```ruby def authorize_user return unless params[:controller] # part of omniauth controller flow verboten! unless params[:controller].include?("devise") || current_user.has_role?(Role::SUPER_ADMIN) || current_user.has_role?(Role::ORG_USER,...

Need a 'graceful landing' if someone goes to a screen they don't have access to while logged in.

Yep, that's what I think we need to do. Now I get why this is happening, but your current fix doesn't go far enough. Your suggestion is what I would...