denny / ShinyCMS

ShinyCMS is an open source CMS. This is the Perl version, built with Catalyst and DBIC. (There is also a Ruby on Rails version: www.github.com/denny/ShinyCMS-ruby)
56 stars 24 forks source link

Numeric shop_item.code breaks; need to remove special case code #62

Closed wbraswell closed 8 years ago

wbraswell commented 9 years ago

When I tried to add an item (“Keyboard”), it failed with Catalyst’s error page being shown. The second time when I tried to add the same item, but with a different item code, again it failed the same way. However, http://dylansworld.net/admin/shop/list-items lists the item (“Keyboard”). Clicking “View” loads a page with the message “Specified item not found. Please try again.”. “Edit” errors out.

...

[error] Caught exception in ShinyCMS::Controller::Admin::Shop->get_item "Can't call method "shop_item_elements" on an undefined value at /home/wbraswell/public_html/dylansworld.net-latest/script/../lib/ShinyCMS/Controller/Admin/Shop.pm line 109.", referer: http://dylansworld.net/admin/shop/add-item

...

you can't use all numeric item codes sorry! oh wow okay that's a holdover from when I was originally coding the shop to allow viewing of items via either their 'sku code' or their database ID well we need to somehow catch that instead of causing a weird error :) eventually I decided to only use the sku (once I started auto-filling it on item creation), but I haven't ripped out all the if/else blocks that look for numeric IDs and handle them specially we don't need to catch it, we need to remove all the code that special cases numeric IDs file an issue on github for that? or just file one for 'numeric shop item ID breaks stuff' and I can fill in details in comments sorry, that's a weird one :)
denny commented 8 years ago

@wbraswell, I'd appreciate it if you could do a little testing on this one at some point; it was a relatively complicated change, lots of bits extracted and tidied up in both front-end and admin-side controllers. I think I got it all right, but a second pair of eyes would be reassuring! :)

wbraswell commented 8 years ago

Okay, I am in the process of working on my servers, at some point in the not-too-distant future I will get an updated copy of Shiny running and test out this issue.