[freeside-devel] Some fixes to review

Ivan Kohler ivan at freeside.biz
Sat Jan 18 22:20:09 PST 2014


Thanks for your contribution!


On Sun, Jan 19, 2014 at 02:06:53AM -0300, Fernando M. Kiernan wrote:
>       - Fix Setup.pm (freeside-setup didn’t finalize OK without this)

Applied this fix but reworked slightly (on master, moving all the "use" 
statements to the beginning; on FREESIDE_3_BRANCH, moving all the use 
statements to the beginning normally).


>       - Fix region selector on SelfService (the selector had problems with States having spaces
> in its name)

Applied (cherry picked).


>       - Fix Ticketing System Permissions. (now you can restrict access to or hide all Ticket
> system for a Freeside User)

This doesn't seem entirely right.

You change everything in menu.html to be based off the RT "Show Ticket" 
or "Superuser".  This makes sense for Reports->Tickets.  I'm not sure it 
makes sense for Tools->Ticketing (don't some of these have their own RT 
permissions?).  It definitely doesn't make sense for 
Configuration->Ticketing, where you replace the existing check for the 
RT right "ShowConfigTab".

You also seem to have change the ticket _search_ to be based on the 
"ShowConfigTab" right, which definitely isn't right.


>       - Fix delete credit access on payment history. (Just a permission check problem that
> didn’t allow deleting a credit)

Applied (cherry picked).


>       - Fix Quick charge page. (when moving from perl 5.10 to 5.14 started to hav problems with
> the last line of the file)

Applied (cherry picked).  Guessing this isn't the perl version but rather perhaps "use 
strict" for HTML::Mason components in your new environment.


>       - Allow en_US on available locales list. (so you can limit languages and allow English
> along others)
>       - Hide en_US on invoicing locales. (en_US takes de global invoicing settings so it
> shouldn’t be listed as an option)

Applied these (cherry-picked).


>       - Limit languages on preferences to only those available-locales. (now you can choose
> only from the available locales and not from all of them)

There seems to be a lot of reformatting noise in this change.  Idea is 
fine; I'd take a clean change.


-- 
_ivan


More information about the freeside-devel mailing list