Bug 17718 - Allow usage of Smartform API (Czech only)
Summary: Allow usage of Smartform API (Czech only)
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Radek Šiman (R-Bit Technology, s.r.o.)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-04 10:36 UTC by Radek Šiman (R-Bit Technology, s.r.o.)
Modified: 2023-06-25 20:58 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 17718: Smartform API allowed in patron registration (13.75 KB, patch)
2016-12-04 18:48 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
Bug 17718: Used Koha.preference instead of passing params to templates from module memberentry.pl (12.80 KB, patch)
2016-12-05 14:33 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
Bug 17718: Fixed QA tool complains (13.53 KB, patch)
2016-12-06 08:40 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17718: Smartform API allowed in patron edit form (16.88 KB, patch)
2016-12-06 09:21 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Radek Šiman (R-Bit Technology, s.r.o.) 2016-12-04 10:36:52 UTC
Enable to use Smartform API to simplify petron registration process namely address entering. This service allows address autocompletion for any address in the Czech Republic including street names and numbers.
Comment 1 Radek Šiman (R-Bit Technology, s.r.o.) 2016-12-04 18:48:50 UTC
Created attachment 57948 [details] [review]
Bug 17718: Smartform API allowed in patron registration

Test plan:
1) Apply the patch
2) Setup preferences "SmartformEnable", "SmartformClientID"
3) Edit a patron
4) Start entering street in an address
5) Select a street from autocomplete
6) Continue entering street number and select the right address
7) ZIP and city should get values from autocomplete
8) Disable SmartformEnable and repeat steps above
9) No autocompletion should occur
Comment 2 Radek Šiman (R-Bit Technology, s.r.o.) 2016-12-04 18:51:43 UTC
Side note: only DE version of address template scripts have changed, because Czech intallations do not need US address version at all.
Comment 3 Josef Moravec 2016-12-05 07:02:03 UTC
Radek, why did you not use template plugin Koha.Preference for getting SmartformEnable syspref?

Is there any demo account on Smartform.cz? Or should I register to pay for this to start testing?
Comment 4 Josef Moravec 2016-12-05 07:08:33 UTC
> Is there any demo account on Smartform.cz? Or should I register to pay for
> this to start testing?

Forgot this question, I sorted it out: It's enough to register here to start using this service https://secure.smartform.cz/admin/user/create
Comment 5 Radek Šiman (R-Bit Technology, s.r.o.) 2016-12-05 13:12:45 UTC
No registration is required to start testing, just use "smartform" as client ID when working on localhost.
Comment 6 Radek Šiman (R-Bit Technology, s.r.o.) 2016-12-05 14:33:30 UTC
Created attachment 57969 [details] [review]
Bug 17718: Used Koha.preference instead of passing params to templates from module memberentry.pl

Test plan:
1) Apply the patch
2) Setup preferences "SmartformEnable", "SmartformClientID"
   (register at smartform.cz or use "smartform" as the client ID)
3) Show/hide streetnumber as desired in BorrowerUnwantedField pref
   (eg. fill in "streetnumber|B_streetnumber")
4) Edit a patron
5) Start entering street in an address field
6) Select a street from autocomplete
7) Continue entering street number and select the right address
8) ZIP and city should get values from autocomplete
9) Disable SmartformEnable and repeat steps above
10) No autocompletion should occur
Comment 7 Radek Šiman (R-Bit Technology, s.r.o.) 2016-12-05 14:36:54 UTC
Address templates allow to use street number merged into one field with the street name or separately as two fields. Autocomplete should work correctly without problems regardless the settings in BorrowerUnwantedField system preference.
Comment 8 Josef Moravec 2016-12-06 06:34:17 UTC
Sorry, QA tool complains:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/member-alt-address-style-de.inc
   FAIL	  tt_valid
		lines 12, 47, 72, 84

 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/member-alt-contact-style-de.inc
   FAIL	  tt_valid
		lines 36, 60, 72

 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-de.inc
   FAIL	  tt_valid
		lines 35, 47, 71, 83

It's related to HTML1 point of coding guidelines I think.

But the functionality is OK so, when you fix this, I am going to sign-off ;)
Comment 9 Radek Šiman (R-Bit Technology, s.r.o.) 2016-12-06 08:40:12 UTC
Created attachment 57994 [details] [review]
Bug 17718: Fixed QA tool complains

I don't fully understand the reason, but I had run the tool without any
errors. Now I did my best according to coding guidelines, I hope.

Test plan:
1) Apply the patch
2) Setup preferences "SmartformEnable", "SmartformClientID"
   (register at smartform.cz or use "smartform" as the client ID)
3) Show/hide streetnumber as desired in BorrowerUnwantedField pref
   (eg. fill in "streetnumber|B_streetnumber")
4) Edit a patron
5) Start entering street in an address field
6) Select a street from autocomplete
7) Continue entering street number and select the right address
8) ZIP and city should get values from autocomplete
9) Disable SmartformEnable and repeat steps above
10) No autocompletion should occur
Comment 10 Josef Moravec 2016-12-06 09:21:10 UTC
Created attachment 57995 [details] [review]
[SIGNED-OFF] Bug 17718: Smartform API allowed in patron edit form

Test plan:
1) Apply the patch
2) Setup preferences "SmartformEnable", "SmartformClientID"
   (register at smartform.cz or use "smartform" as the client ID)
3) Show/hide streetnumber as desired in BorrowerUnwantedField pref
   (eg. fill in "streetnumber|B_streetnumber")
4) Edit a patron
5) Start entering street in an address field
6) Select a street from autocomplete
7) Continue entering street number and select the right address
8) ZIP and city should get values from autocomplete
9) Disable SmartformEnable and repeat steps above
10) No autocompletion should occur

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 11 Josef Moravec 2016-12-06 09:22:48 UTC
I squashed the patches to simplify history
Comment 12 Jonathan Druart 2016-12-27 17:03:12 UTC
I have enabled SmartformEnable, set SmartformClientID to "smartform", I create a new patron, fill address input with some random chars, but no autocompletion (and no request to the ws).
Can you double-check or tell me what I missed?
Comment 13 Radek Šiman (R-Bit Technology, s.r.o.) 2016-12-27 19:34:34 UTC
Jonathan, were you running it from localhost and using HTTP URL from a local IP? If not, it may not run. The service has some security limitations, because it's paid for business, but any Czech library can use it for free after registration.
Comment 14 Marcel de Rooy 2017-03-10 09:45:45 UTC
Few questions:
Why not only have one preference? If the client id is empty, the pref is obviously not enabled. We already have so much preferences ;)

From a security perspective, I do not really like to include code like this:
<script type="text/javascript" src="http://smartform.cz/api/v1/smartform.js"></script>
Can we do it some other way? Add it to a local lib directory, and have more control?

In general: Can you convince me why we should push it in this current form? I could imagine that we could have a more general 'plugin' that operates on address fields and allows access to selected web services, or so?

Anyway, thanks for your work ! Your answers or adjustments may bring this further.

Changing status to reflect need for feedback.
Comment 15 Radek Šiman (R-Bit Technology, s.r.o.) 2017-03-10 13:28:37 UTC
One preference is a good idea, I will change it.

I understand the security reasons about cross-site inclusion of javascripts. The problem is, this a 3rd party service we really like and we trust it, because it's widely used in our country including a leader of IT market. Not a good reason, I know. Currently they offer direct linkage only. I am not sure if they will ever allow to download the sources as a package. It is connected to their database structure and so they have a good control over compatibility issues, I think. However, I'll try to ask them for the sources. For your information, please look at
http://www.smartform.cz/index.html
and you will see how the service grows on the graph in the right bottom corner of the page.

The only reason to ask for the current form is a growing community of this service users in the Czech Republic. Originally we had an idea similar to the Norwegian patron database and we planned this feature for our country only. On the other hand, you are right that kind of general plugin or driver would be useful, but it highly exceeds the original limits.
Comment 16 Katrin Fischer 2023-06-25 20:58:20 UTC
> 
> In general: Can you convince me why we should push it in this current form?
> I could imagine that we could have a more general 'plugin' that operates on
> address fields and allows access to selected web services, or so?
> 

As this is quite a specific API that might change, this seems like an ideal use case for a plugin. And the plugin system has been improved quite a bit since this bug was originally filed in 2016.