Add support for displaying mobile phone number in profile screen… #1050
Conversation
…ager: select phone numbers to display by their types instead of randomly selecting the last one.
There was a problem hiding this comment.
This works for me, but one suggestion before merging....
Also, we should probably add a value to the Demo driver to match, and update the driver spec in the wiki. I can take care of those things if you do not have time; I'm most interested in your opinion on my suggested adjustment.
| $patron['phone'] = utf8_encode($row['PHONE_NUMBER']); | ||
| } elseif ('Mobile' === $row['PHONE_DESC']) { | ||
| $patron['mobile_phone'] = utf8_encode($row['PHONE_NUMBER']); | ||
| } |
There was a problem hiding this comment.
How would you feel about adding an elseif (!isset($patron['phone'])) here? This way, the 'phone' value prefers the 'Primary' value if set, but if a user has only an "Other" value (or if a library has customized the phone number types to something unexpected) this change doesn't end up hiding phone numbers completely.
There was a problem hiding this comment.
The problem here is that some libraries have used the 'Other number' as something different from a phone number altogether (such as a student number), so it should never be displayed. I'm not sure if they have a primary phone number in that case. It's of course possible to add a "display other phone number" setting too, but do you think this is a real need?
There was a problem hiding this comment.
My concern is not so much about displaying the "other number" as making sure that we display something. Before making this change, the code is going to display whichever phone number comes last in the database, regardless of its registered type. That is obviously less desirable than what you are proposing. I'd just like to have a safety net so that we don't stop displaying values if they don't conform to our expectations -- what I was proposing would basically fail back to the current behavior in situations where weird values were encountered... but as long as a phone number is marked as primary, it would be smart enough to prefer that value and block out the "other" number. Does that make any sense?
There was a problem hiding this comment.
Makes sense, but we need to block out the "other" number regardless of whether there's a primary phone number.
Perhaps a setting for what's displayed as the primary and mobile phone numbers would cater all needs, with these are the defaults:
[Profile]
primary_phone_type = "Primary"
mobile_phone_type = "Mobile"
This would also allow you to display just the mobile phone or whatever you want. What do you think?
There was a problem hiding this comment.
I guess my thinking was that if there's a primary number, it would overwrite the other number, thus suppressing it... and if there's only an other number, displaying that might be better than nothing. But ultimately this is probably not an issue to lose a lot of sleep over, given that it has very little impact on anything. If you'd be more comfortable with the configurable [Profile] settings, that would certainly be fine with me, and we could worry about these edge cases if we ever discover that anyone actually cares about them. ;-)
There was a problem hiding this comment.
I understand, but in our case we really need to make sure the "Other" number is never displayed. So the latest version now has the configuration options. We can of course do this locally too if you don't feel comfortable with it.
There was a problem hiding this comment.
No, it's fine. I'll likely merge later today if testing goes smoothly.
|
The latest commit adds support for defining which phone number type to use as primary and mobile. |
…vocabularies Style fixes for metadata vocabularies branch.
and for Voyager, select phone numbers to display by their type instead of randomly selecting the last one.