Bug 9383 - Bad logic, missing params, and irregular labeling for Members scripts
Summary: Bad logic, missing params, and irregular labeling for Members scripts
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: David Cook
QA Contact:
URL:
Keywords:
: 9470 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-01-14 05:10 UTC by David Cook
Modified: 2016-12-05 21:22 UTC (History)
4 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 9383 - Bad logic, missing params, and irregular labeling for Members scripts (9.07 KB, patch)
2013-01-14 05:57 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2013-01-14 05:10:06 UTC
1) Category_type not being passed from moremember.pl to applicable templates

2) Logic in moremembers.pl doesn't work without category_type param. 

However, with the param, it runs category_type conditions one after another (if/else, if/else), rather than nesting them (if/elsif/else), which creates duplicate data for the P category type. 

3) Labels are inconsistent between memberentrygen.pl page and moremember.pl page
Comment 1 David Cook 2013-01-14 05:57:12 UTC
Created attachment 14552 [details] [review]
Bug 9383 - Bad logic, missing params, and irregular labeling for Members scripts

To test:

Before patch:

1) Add new patrons with "organization", "adult", and "professional" category types/codes.
2) Observe that they all have input boxes for Primary phone, Secondary phone, and Other phone.
3) Also, observe that they all have input boxes for Primary email, and Secondary email
4) Save the patron and look at the Details page that should come up automatically
5) Note that your "Secondary Phone" now shows up as "Other Phone" and your "Other phone" shows up as "Secondary Phone". 

---

After Patch:

1) Organization will have input boxes for "Organization phone" and "Organization" email, which correspond to the (Primary) phone/email fields.

2) Professionals will have input boxes for Primary/Secondary phones (but not Mobile phones) which correspond to phone/phonepro DB fields.

3) Other patrons will have input boxes for Primary/Secondary/Mobile, which correspond to phone, phonepro, mobile DB fields.

---

Personally, I don't see why Professionals should be separated from the rest of patrons (other than Organizations). In fact, I don't know if there is even a point separating Organizations! I think Primary/Secondary/Mobile are fine clear across the board.

So maybe a follow-up to this would actually eliminate the conditions for "I" (organization) and "P" (professional) while retaining the consistent labelling between the memberentrygen.pl and moremember.pl files.
Comment 2 Owen Leonard 2013-02-05 15:46:55 UTC
(In reply to comment #1)

> Personally, I don't see why Professionals should be separated from the rest
> of patrons (other than Organizations). In fact, I don't know if there is
> even a point separating Organizations! I think Primary/Secondary/Mobile are
> fine clear across the board.

Professional type patrons can be created with links to specific Organizations. I don't know much about how this works in practice but it's obviously a feature which was created with specific intent.
Comment 3 David Cook 2013-02-05 23:05:44 UTC
(In reply to comment #2)
> (In reply to comment #1)
> 
> > Personally, I don't see why Professionals should be separated from the rest
> > of patrons (other than Organizations). In fact, I don't know if there is
> > even a point separating Organizations! I think Primary/Secondary/Mobile are
> > fine clear across the board.
> 
> Professional type patrons can be created with links to specific
> Organizations. I don't know much about how this works in practice but it's
> obviously a feature which was created with specific intent.

I agree, Owen, but I was referring to the separation of phone/email labels rather than the actual patron types. I don't know why an organisation would need "organisation phone" rather than "primary phone" or why a professional would have different labels for the same fields as any other individual patron. 

I think there might be some disagreement about which labels I've chosen to apply in this patch (like using Mobile Phone for mobilephone, as I think some people prefer to use Other phone as mobilephone might actually be the Primary Phone). However, the current templates don't actually work to differentiate them anyways at the moment, because the categorycode param isn't passed to the template from the perl. 

Besides that, the labels appear to be inconsistent between the patron entry page and the patron detail page. I imagine it would be disconcerting to put your work phone as your primary phone and then have your home phone show up as your primary phone on your page. (Mind you, that's awfully detail-oriented, so maybe most people wouldn't notice.)
Comment 4 Owen Leonard 2013-02-06 15:11:51 UTC
*** Bug 9470 has been marked as a duplicate of this bug. ***
Comment 5 Owen Leonard 2013-02-06 15:48:12 UTC
What does this change to memberentry

+    "$category_type"  => 1, #passes category type (e.g. C, A, S, P, I, X) to template like in moremember.pl do?
Comment 6 David Cook 2013-02-06 22:51:01 UTC
(In reply to comment #5)
> What does this change to memberentry
> 
> +    "$category_type"  => 1, #passes category type (e.g. C, A, S, P, I, X)
> to template like in moremember.pl do?

That looks like a bit of a typo. It should say to templates like moremember.tt, moremember-brief.tt, or memberentrygen.tt

As for what it does, it passes the patron's category type (in the case of an organization: I, in the case of a professional: P) to the template to determine which phone/email labels to apply. 

For memberentrygen.tt: 

If I, "Organization Phone:" (phone) and "Organization Email:" (email), and no emailpro, no mobile phone, no phonepro

If P, "Primary Phone" (phone), "Secondary Phone" (phonepro), "Primary Email" (email), "Secondary Email" (emailpro), and no mobile phone

All other category types get the default labels (which are also mapped consistently over to the moremember.tt templates).
Comment 7 Owen Leonard 2013-03-14 13:31:53 UTC
Looking again at these changes and trying to figure out the logic of which fields should be displayed for which types makes me think you're right in wondering why we try to change the display for different patron types.

Maybe we should take a step back and try to isolate what differences are really necessary and what differences might have originated with some library's specific needs.

Some starting assumptions:

- Any category type could easily have any kind of contact information.
- Child patrons and Professional patrons need to show guarantor information
- Organizations should not have the same name fields humans have

Another idea: Making  BorrowerUnwantedField and  BorrowerMandatoryField configurable at the patron-category level would offer libraries better control over the decisions that the template currently makes for them.
Comment 8 David Cook 2014-08-20 05:42:06 UTC
This bug partially solved by bug 12407
Comment 9 David Cook 2016-05-16 00:09:16 UTC
I'm closing this one, as it's super old and I reckon it's been fixed or is no longer relevant in its current incarnation.