sahana / vesuvius

Sahana Vesuvius
http://sahanafoundation.org/products/vesuvius/
MIT License
24 stars 27 forks source link

'Edit A Person' form is not provided in systems where curl is not installed #8

Closed ravihansa3000 closed 10 years ago

ravihansa3000 commented 10 years ago

EAP module's 'Edit A Person' form is not provided in systems where curl is not installed. Does not produce an error/warning to gracefully handle. Just stops the form visibility to the user.


Imported from Launchpad using lp2gh.

ravihansa3000 commented 10 years ago

(by rasade88) Can confirm: EAP does not work on systems where CURL is not installed.

ravihansa3000 commented 10 years ago

(by kaushikaruk) In eap/main.inc, it creates an object of goo_gl class to create a short URL. In the constructor it calls curl_init() so curl should be pre installed. In the Fix I added a condition to check whether curl is installed or not.

Here I attached two patches for two solutions.

Bug_1172272_1.patch - Giving ability to edit the person record but short url will not be shown. Bug_1172272_2.patch - Giving an error message informing user to install curl to proceed.

ravihansa3000 commented 10 years ago

(by kaushikaruk) PFA Bug_1172272_2.patch - Giving an error message informing user to install curl to proceed.

ravihansa3000 commented 10 years ago

(by rasade88) Hi Rukshan,

I have applied the first patch. But would like to see a hybrid, with a warning being displayed if CURL is not installed, but the module workflow proceeding forward anyway.

ravihansa3000 commented 10 years ago

(by ravihansa3000) Added a function to check whether 'curl' is present in the existing php configuration: _is_curl_installed() Separate function might come in handy since there are many instances of curl_init calls. Used add_error function to display the error message and gracefully handle the exception.

ravihansa3000 commented 10 years ago

(by ravihansa3000) Implementation of function _is_curl_installed()

ravihansa3000 commented 10 years ago

(by kaushikaruk) Here is the patch for hybrid solution. I revert the changes of the 1st patch and created a new one.

ravihansa3000 commented 10 years ago

(by rasade88) Committed Kaushika's fix, thanks.