Default currency

I've also noticed that room_search always exposes rates with Dollar sign [$], even if we set a different currency.

My suggestion [for next release?] is :
replace $ sign inside hotel_booking.module with variable_get('uc_currency_sign', '€')

hope this helps

Thank you Augusto.

I went through the hotel_booking.module searching for $ (that is, where $ is a currency sign and not the beginning of an array name, etc).

Here is my list of substitutions for hotel_booking.module in release 6.x-1.0-beta3:

Replace '$' (including the ') with variable_get('uc_currency_sign', '€') in the following lines:
1406
1409
1418
1431
1625
1628
1637
1650

Replace $0.00 with €0.00 in the following lines:
1037
1046
1084
1093

Replace $ ' (that is $ space ') with '. variable_get('uc_currency_sign', '€') . ' ' in the following lines:
602
614
615
616

Replace $ " (that is $ space ") with " . variable_get('uc_currency_sign', '€') . ' ' in the following lines:
625

I attach the following files:
- hotel_booking.module.6.x-1.0-beta3.before.txt, that is the same hotel_booking.module file that is distributed with release 6.x-1.0-beta3
- hotel_booking.module.6.x-1.0-beta3.after.txt, that is the hotel_booking.module file after replacing the $ currency signs with € currency signs

It would be nice to find a way of dynamically substitute the currency sign with the default currency, as set in Ubercart Store administration > Store settings > Edit > Format settings > Currency format.

I hope that my note will help.

Ivan

ivanpellegrin
-------------------------
http://artdesejour.com
http://maestridellarte.com
http://xglot.com
http://biartisans.com (business intelligence artisans)
http://nakedbiguy.com (the naked truth on business intelligence)
http://ivanpellegrin.com

There is a better way to do this.

Your solution only solves part of the problem, but doesn't account for when the currency sign is after the number.

This still isn't the perfect solution, but would work better:

Add this to the top of each function that outputs currency values

$sign_flag = variable_get('uc_sign_after_amount', FALSE);
    $currency_sign = variable_get('uc_currency_sign', '$');

Then, wherever there's a currency value being output, do it as such:

$output .= ($sign_flag) ? $currency . $currency_sign : $currency_sign . $currency;

This can most likely be done with uc_currency_format(), but there's a lot of places that prices are displayed rounded up to the nearest whole number (primarily for styling ease)

All of the prices should go through uc_price($price, array('revision' => 'formatted')) where $price is your price, this allows other handlers to do their job as required.

Thank you Will, thank you Lee

I have taken Lee's suggestion and digged into the uc_price function.

I have then applied it to everywhere in the uc_hotel module where I encountered a $ currency sign. The list is in fact longer than in my previous entry. So far I scanned through all the .module files. This is the list of modifications that I have performed (Please notice that I substituted $price with whatever code was already in the .module files representing the "price" to be formatted with the currency sign):

1.

hotel_booking.module

Replace '$' (including the ') with uc_price($price, array('revision' => 'formatted')) in the following lines:
1406 OK
1409 OK
1418 OK
1431 OK
1625 OK
1628 OK
1637 OK
1650 OK

Replace $0.00 with uc_price(0, array('revision' => 'formatted')) in the following lines:
1037 OK
1046 OK
1084 OK
1093 OK

Replace $ ' (that is $ space ') with uc_price($price, array('revision' => 'formatted')) in the following lines:
602 OK
614 OK
615 OK
616 OK

Replace $ " (that is $ space ") with uc_price($price, array('revision' => 'formatted')) in the following lines:
625 OK

2.

hotel_room_type.module

Replace '$' with uc_price($price, array('revision' => 'formatted')) in line:
360 OK

3.

hotel_calendars.module

Replace '$' with '' in line:
553 OK

This part needs better coding.
I was trying to use the 'prec' option in the uc_price funtion but it didn't seem to work
I was getting all the time decimal point and two digits after it.
That in turn was making the table cells too wide, and I was getting two months on the first row, and one month in the second row, two months in the third row, one month in the forth row, etc.
Now the calendar view display only an integer without the currency sign, which is better than displaying the wrong currency sign.
Perhaps someone knows how to use the uc_price function to format with the currency and without decimals?

4.

hotel_addons_upgrades.module

Nothing to replace

----------

Please disregard, the OK's, that just means that I have checked each line twice for syntax and logic.

I attach the new files (with a .txt extension to be stripped). Please notice that I have worked on the 6.x-1.0-beta3 release.

I have also just applied for a CVS account. When I receive it, I will produce the patch in the appropriate format, so that it can be tested by the community.

ivanpellegrin
http://artdesejour.com
http://maestridellarte.com
http://xglot.com
http://biartisans.com (business intelligence artisans)
http://nakedbiguy.com (the naked truth on business intelligence)
http://ivanpellegrin.com

Thanks Ivan
This has been on our to do list for sometime.
You don't need CVS access to make a patch, you can checkout the module using an anonymous account and create a patch.
The beta release and the dev (HEAD) release have some differences but the patch should be close enough with some manual tinkering.
Note also that the latest dev release has full support for translation so if we can get currency and translation support then I'd say this warrants another beta release.

Thank you Lee

I have not received my CVS access yet, so I went through a more long winded method to create the patches
=> Perhaps you can put a good word for my CVS access at http://drupal.org/node/801376 please?

• I downloaded the latest 6.x-1.x-dev release and modified those files (that was done with the dev date of 2010-May-15)
• I performed the same type of changes as before, but now the line numbers are slightly different.Also some of the lines have changed between 6.x-1.0-beta3 and 6.x-1.x-dev: I made sure to respect the changes in 6.x-1.x-dev.
• Finally I created the patches

This is my list of activities, that can be read as follows: The line number x in 6.x-1.0-beta3 has become line number y in 6.x-1.x-dev and it has (no) differences. The OK means that I checked twice, which should not be interpreted as I did not make any mistake, but any mistake is made systematically throughout :)

1.

hotel_booking.module

Replace '$' (including the ') with uc_price($price, array('revision' => 'formatted')) in the following lines:
1406 -> 1413 : diff : OK
1409 -> 1416 : no diff : OK
1418 -> 1425 : no diff : OK
1431 -> 1438 : no diff : OK
1625 -> 1629 : diff : OK
1628 -> 1632 : no diff : OK
1637 -> 1641 : no diff : OK
1650 -> 1654 : no diff : OK

Replace $0.00 with uc_price(0, array('revision' => 'formatted')) in the following lines:
1037 -> 1033 : no diff : OK
1046 -> 1042 : no diff : OK
1084 -> 1080 : no diff : OK
1093 -> 1089 : no diff : OK

Replace $ ' (that is $ space ') with uc_price($price, array('revision' => 'formatted')) in the following lines:
602 -> 598 : no diff : OK
614 -> 612 : no diff : OK
615 -> 613 : diff : OK
616 -> 614 : diff : OK

Replace $ " (that is $ space ") with uc_price($price, array('revision' => 'formatted')) in the following lines:
625 -> 623 : no diff : OK

2.

hotel_room_type.module

Replace '$' with uc_price($price, array('revision' => 'formatted')) in line:
360 -> 341 : no diff : OK

3.

hotel_calendars.module

Replace '$' with '' in line:
553 -> 558 : no diff : OK

This part needs better coding.
I was trying to use the 'prec' option in the uc_price funtion but it didn't seem to work
I was getting all the time decimal point and two digits after it.
That in turn was making the table cells too wide, and I was getting two months on the first row, and one month in the second row, two months in the third row, one month in the forth row, etc.
Now the calendar view display only an integer without the currency sign, which is better than displaying the wrong currency sign.
Perhaps someone knows how to use the uc_price function to format with the currency and without decimals?

4.

hotel_addons_upgrades.module

Nothing to replace

-----

I attach the three patches - one for each .module file - and a tgz containing the actual .module files after the changes.

Let me know if this helps

ivanpellegrin
http://artdesejour.com
http://maestridellarte.com
http://xglot.com
http://biartisans.com (business intelligence artisans)
http://nakedbiguy.com (the naked truth on business intelligence)
http://ivanpellegrin.com

Just an afterthought

Do I have to apply to be a co-maintainer in order to offer patches?

Should I anyway?

It is likely that I will participate closely to this project, firstly because I'm about to open my own hotel in Brussels in August.

ivanpellegrin
http://artdesejour.com
http://maestridellarte.com
http://xglot.com
http://biartisans.com (business intelligence artisans)
http://nakedbiguy.com (the naked truth on business intelligence)
http://ivanpellegrin.com

Hi Ivan
Firstly thanks for your work here. I will review it first chance I get which is most likely to be next week.
Secondly, no you don't need CVS access or to be a co-maintainer to create and post patches, check out my Drupal account and you'll see I've posted heaps of patches for various modules that I've not got commit CVS access for.
Thirdly, no offence intended, you are unlikely to get CVS commit access to a module on the basis of one issue/patch. since commit access to a module gives you universal privileges to the module, I'm not willing to agree to co-maintainer/commit access until I know you better and we have built up a degree of trust and shared understanding of what we want the module to do/become. Will and I developed this over two to three months while we were both working towards a shared goal and I'm sure that if you have a long term interest in and contribution to the project then a time will come when Will and I feel the same way towards you.
If you read this Will, feel free to add your 2c
Thanks again Ivan
Lee

I agree, and trust your judgement Lee. :)

Your patches do not apply. Well, the hotel_booking.module patch doesn't anyway..

Here's a unified patch, should apply against the current cvs head, as of right now. Kind of weird how it is written though, it basically removes every line of hotel_booking.module, and then rewrites the whole file.. The other two affected files only have a single line changing.

I'm guessing something must have changed in HEAD and hadn't yet been repackaged for the dev release. This is why it's always a better option to use cvs, plus it's dead simple to make a patch from a cvs checkout, you make your changes and run cvs diff -up > [patch_file_name]

And as far as the instructions go for checking out from cvs, this will always show you how to get the HEAD version, if nothing else.. it's possible to get other releases this way as well, but you have to know the proper revision tag to insert into the checkout command. Anyway, every project on drupal.org should have a CVS Instructions tab, as does this project: http://drupal.org/project/uc_hotel/cvs-instructions

Thank you Will, those are the commands that I was looking for. That procedure is much easier for producing patches.

No offence Lee. I had erroneously understood that in order to submit a patch I had to apply to be a co-maintainer.

I have now recreated the patch with the procedure pointed out by Will.

I also see what you are saying. It seems that it removes every line of hotel_booking.module and then it rewrites the whole file.

When I view the patch file with vi all the lines that are being removed have a ^M at the end: isn't that CR in DOS? Could it be that hotel_booking.module in the latest dev was uploaded with the wrong transfer option (or something on that line of thought)?

hotel_booking.module is changed in 17 lines. The other two files are only changed in one line each - that is correct.

In the meantime I upload the same patch for your attention. To me it seems the same as yours, apart from the headers.

Could you check if I prepared it correctly please? It is my first contribution and I might have missed something. It would save time for the next.

Hi, I was wondering if this issue fixed in the current dev version? I assume the patch above will, due to its age, not work with the latest version.