Hi Marc,

Applied the patch on Sandbox 14. 

- Logged in to OPAC
  - with blocked /debarred user (with and without comment and date)
  - with user who has his card marked as lost
  - with user with an account that is about to expire
  - with user with expired account
  - with user with uncertain address information
  - with user who has got a circulation message (from checkout page)
  - with user who has a OPAC note
- Verify that information is correct and displays nicely.  I did not find that  links in messages (for example to books in the OPAC) worked.

- Attempted to test with user who has to much fines. Created manual fines/lost items charges, but fines did NOT display in "total due".

On another sandbox, I did not apply the patch and found that for manual restriction, the expiration date picker did NOT pop up. Was fixing this part of your patch?

Hi Chris,

Thanks for testing!

Hmm, you talk about date picker and links to books, did you really test the OPAC summary page?
Home > [Patron's name] > Your summary
[server]/cgi-bin/koha/opac-user.pl

It is the page that appears immediately after a log in to the OPAC ( at the left the tab 'your summary').

After log in you should see messages like the following (depending on the user's status):
----
 Please note: Your account has expired as of 15.12.2015. Please contact the library if you wish to renew your account.

    Please note: According to our records, we don't have up-to-date contact information. Please contact the library. Update your contact information (Please note: there may be a delay in restoring your account if you submit online.)
    Please note: Your library card has been marked as lost or stolen. If this is an error, please contact the library.
    Please note: You cannot renew your books online. Reason: Your fines exceed 100.00. Please pay your fines if you wish to renew your books. Go to your account page
----

Note: To test for a user who has to much fines, you can

- go to staff client

- set syspref 'OPACFineNoRenewals' e.g. to 100 
  (if empty nothing is calculated)

- add a manual fine for this user, e.g. 200
  (Search a user, 'Fines' in lefthand menue, 
   Tab 'Create manual invoice')

Testing with a user who has too many fines did not work. I had the syspref set correctly and created a manual invoice of $100 but there was no error message saying the account was blocked due to fines. A 'Fines' tab showed up saying 'Fines (100)', which I think would be better to say $100.00.
Didn't see my circulation message, not sure how this is meant to affect the OPAC?

(In reply to Aleisha Amohia from comment #5)
> Testing with a user who has too many fines did not work. I had the syspref
> set correctly and created a manual invoice of $100 but there was no error
> message saying the account was blocked due to fines. A 'Fines' tab showed up
> saying 'Fines (100)', which I think would be better to say $100.00.
> Didn't see my circulation message, not sure how this is meant to affect the
> OPAC?

Hi Aleisha, thanks for testing.

- I amended the patch to format fines tab as well. (100.00 instead of 100). Regarding the display of the currency sign see Bug 4078 (waiting for Sign-off)

Regarding a user who has too many fines:
- I re-tested with syspref OPACFineNoRenewals set to 100 and a patron 'Tester' with fines (200) and logged in to the OPAC with this user. (Expecting: A message that now renewals are possible due to fines.)

Without patch, I get after log-in (cgi-bin/koha/opac-user.pl):
- Warning message:
Please note:  Since you have  more than 100.00 in fines, you cannot renew your books online. Please pay your fines if you wish to renew your books.

- Tab Heading:
Fines (200.00)

With patch, I get:
- Warning message:
Please note: You cannot renew your books online. Reason: Your fines exceed 100.00. Please pay your fines if you wish to renew your books. Go to your account page
[As expected. Better wording / no sentence splitting for translatability]

- Tab Heading:
Fines: 200.00

Note: I amended "Bug 4078 - Display active currency symbol on currency output and input" in a way that it can be tested on top of this bug. (But it does not depend on this bug nor blocks this bug).

> With patch, I get:
> - Warning message:
> Please note: You cannot renew your books online. Reason: Your fines exceed
> 100.00. Please pay your fines if you wish to renew your books. Go to your
> account page
> [As expected. Better wording / no sentence splitting for translatability]
> 
> - Tab Heading:
> Fines: 200.00

The fines tab looks much better now but I'm still not seeing any warning message when I log in. I get a warning message when I try to place a hold on something but not on the summary page when I log in.

Also I tested with your patch for Bug 4078 and it works well, so can I sign off on that one?

(In reply to Aleisha Amohia from comment #9)

>(...)I'm still not seeing any warning message when I log in. 

I can reproduce the behaviour. It is related to a system preference

OpacRenewalAllowed: [Allow] patrons to renew their own books on the OPAC.

CONFLICT (content): Merge conflict in opac/opac-user.pl
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt

Marc, I have not tested the patch, but it seems that a conflict with bug 15344 has not been fixed correctly.
BORROWER_INFO should not be BORROWER_INF

Does not apply...

Should be fixed on top of 14930

Comment on attachment 47626 [details] [review]
Bug 15374 - Translatability: Fix issues on OPAC summary page

Oh, the latest patch contains merge markers (<<<<<<< HEAD etc)

Applying: Bug 15374 - Translatability: Fix issues on OPAC summary page
fatal: sha1 information is lacking or useless (opac/opac-user.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 15374 - Translatability: Fix issues on OPAC summary page

Hi Aleisha,

I just re-tested. PThe patch applies fine on top of Bug 14930.

(In reply to Marc Véron from comment #22)
> Hi Aleisha,
> 
> I just re-tested. PThe patch applies fine on top of Bug 14930.

Oops I completely forgot to apply the dependency (silly me). Will retest

There are a couple of problems with the patch.

When the user has too many fines, there is no message on the summary page to say the user is blocked from check outs.
Also, small typo: When the account has a restriction, the message says "If shows your account to be clear", which doesn't make sense. Perhaps change to "If your account is shown to be clear"?

Hmm, testing...  Seems OK to me...

- Syspref OpacRenewalAllowed: Allow
- Syspref OPACFineNoRenewals: 2
- Patron has fines: Total due: 	5.01

- Log-in to Opac with patron, go to 'your summary'
Result: "Please note: You cannot renew your books online. Reason: Your fines exceed 2.00. Please pay your fines if you wish to renew your books. Go to your account page"
OK

- Syspref OPACFineNoRenewals: 10
- Patron has fines: Total due: 	5.01
- Refresh 'your summary'
Result: No message
OK


- Syspref OPACFineNoRenewals: 10
- Add manual fine for patron (10). Now total due is: 	15.01
- Refresh 'your summary'
Result: "Please note: You cannot renew your books online. Reason: Your fines exceed 10.00. Please pay your fines if you wish to renew your books. Go to your account page"
OK

- Syspref OPACFineNoRenewals: 0
- Patron has fines: Total due: 	15.01
- Refresh 'your summary'
Result: "Please note: You cannot renew your books online. Reason: You have fines. Please pay your fines if you wish to renew your books. Go to your account page"
OK

- Syspref OPACFineNoRenewals: (empty)
- Refresh 'your summary'
Result: No message
OK

- Syspref OPACFineNoRenewals: xyndjeksfjasdf 
  (non-numeric, is not validated by syspref form)
- Refresh 'your summary'
Result: No message
OK

- Syspref OPACFineNoRenewals: (one space char) 
  (non-numeric, is not validated by syspref form)
- Refresh 'your summary'
Result: No message
OK

Hi Aleisha,
Thank you very much for continuous testing. 

I could not reproduce your issue from comment #24, see my tests in comment #25. If I missed something, please provide information on how you got your result.

I attached a patch with fixed wording.

(In reply to Marc Véron from comment #27)
> Hi Aleisha,
> Thank you very much for continuous testing. 
> 
> I could not reproduce your issue from comment #24, see my tests in comment
> #25. If I missed something, please provide information on how you got your
> result.
> 
Hi Marc,

I mean that when a user has been blocked from checkouts because their fine balance is over the limit, I feel like a message should show up saying this, regardless of if OpacRenewalAllowed is set to Allow or not.

Created attachment 47772 [details] [review]
Bug 15374 - Translatability: Fix issues on OPAC summary page

Fix several issues on opac-user.tt with sentences splitted by tags,
leading to odd translations.

Additionally, fix a broken link that should point to opac-account.pl

To test:
- Apply patch
- Log in to OPAC
  - with blocked /debarred user (with and without comment and date)
  - with user who has to much fines
  - with user who has his card marked as lost
  - with user with an account that is about to expire
  - with user with expired account
  - with user with uncertain address information
  - with user who has got a circulation message (from checkout page)
  - with user who has a OPAC note
- Verify that information is correct and displays nicely and that links
  in messages work
- Carefully examine code
- Search for regressions

(Amended to add price formatting in fines tab label)

Can see the warning message now :) signing off

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>

(Amended to make patches apply, see comment #12)

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>

(Amended to make it apply and to fix BORROWER.INF -> BORROWER->INFO.
 Needs new sign-off. / MV)
(Amended to fix typo from comment #24 / mv)

Amended to fix merge conflict

(In reply to Aleisha Amohia from comment #28)
> (In reply to Marc Véron from comment #27)
> > Hi Aleisha,
> > Thank you very much for continuous testing. 
> > 
> > I could not reproduce your issue from comment #24, see my tests in comment
> > #25. If I missed something, please provide information on how you got your
> > result.
> > 
> Hi Marc,
> 
> I mean that when a user has been blocked from checkouts because their fine
> balance is over the limit, I feel like a message should show up saying this,
> regardless of if OpacRenewalAllowed is set to Allow or not.

Hi Aleisha,

Is that something that changes with this path ( = a regression) or is the behaviour the same without patch?

- If it is a regression, please fail the patch

- If the behaviour is the same with and without patch, I propose to file a new Bug on top of this one.

(In reply to Marc Véron from comment #30)
> Hi Aleisha,
> 
> Is that something that changes with this path ( = a regression) or is the
> behaviour the same without patch?
> 
> - If it is a regression, please fail the patch
> 
> - If the behaviour is the same with and without patch, I propose to file a
> new Bug on top of this one.

Ah yes I understand now. The behaviour is the same with and without the patch so I will file a new bug and sign off on this one now.

Sorry for messing you around a bit!

(In reply to Aleisha Amohia from comment #31)
> (In reply to Marc Véron from comment #30)
> > Hi Aleisha,
> > 
> > Is that something that changes with this path ( = a regression) or is the
> > behaviour the same without patch?
> > 
> > - If it is a regression, please fail the patch
> > 
> > - If the behaviour is the same with and without patch, I propose to file a
> > new Bug on top of this one.
> 
> Ah yes I understand now. The behaviour is the same with and without the
> patch so I will file a new bug and sign off on this one now.
> 
> Sorry for messing you around a bit!

Hi Aleisha,
Thanks for SO and no problem - somtimes to much trees prevent us to see the forest, as a German proverb says... 
:-)

Comment on attachment 47772 [details] [review]
Bug 15374 - Translatability: Fix issues on OPAC summary page

Review of attachment 47772 [details] [review]:
-----------------------------------------------------------------

::: opac/opac-user.pl
@@ +123,5 @@
>      $borr->{'amountoutstanding'} = -1 * ( $borr->{'amountoutstanding'} );
>  }
>  
> +my @bordat;
> +$bordat[0] = $borr;

hum?
Looks like a merge conflict issue.

Created attachment 47842 [details] [review]
Bug 15374: (followup) Remove 2 obsolete lines from opac-user.pl

This patch removes 2 obsolete lines from opac-user.pl, see comment #33

I give up this bug in favor of Bug 15775.
Maybe some of the code here can then be used on top of 15775.