|
Description
Jonathan Druart
2020-04-06 12:54:15 UTC
Created attachment 102439 [details] [review] Bug 25070: Centralize member-alt-address-style There are 5 includes files per value of AddressFormat (us, de, fr). The code is duplicated for each language, the only thing that changes is the order. Wwith this patchset we are going to refactore them to avoid further regressions. The idea is to remove all code that is duplicated and remove the need of having 5*3 files. We are going to limit them to 5 files, 1 per use. Note that we could go even further, for instance member-display-alt-address-style and member-display-address-style are quite identical. Test plan (for the whole patchset): - Create a patron with all the address, contact and alternate info filled - Edit them and save - Look at the patron's info (top left) in the patron module, and the patron detail view. - Play with the different values of AddressFormat The order and the info must be displayed identically before and after this patchset. No change is expected. Created attachment 102440 [details] [review] Bug 25070: Centralize member-alt-contact-style Created attachment 102441 [details] [review] Bug 25070: Centralize member-display-address-style Created attachment 102442 [details] [review] Bug 25070: Centralize member-display-alt-address-style Created attachment 102443 [details] [review] Bug 25070: Centralize member-main-address-style Created attachment 102477 [details] [review] Bug 25070: Centralize member-alt-address-style There are 5 includes files per value of AddressFormat (us, de, fr). The code is duplicated for each language, the only thing that changes is the order. Wwith this patchset we are going to refactore them to avoid further regressions. The idea is to remove all code that is duplicated and remove the need of having 5*3 files. We are going to limit them to 5 files, 1 per use. Note that we could go even further, for instance member-display-alt-address-style and member-display-address-style are quite identical. Test plan (for the whole patchset): - Create a patron with all the address, contact and alternate info filled - Edit them and save - Look at the patron's info (top left) in the patron module, and the patron detail view. - Play with the different values of AddressFormat The order and the info must be displayed identically before and after this patchset. No change is expected. Created attachment 102478 [details] [review] Bug 25070: Centralize member-alt-contact-style Created attachment 102479 [details] [review] Bug 25070: Centralize member-display-address-style Created attachment 102480 [details] [review] Bug 25070: Centralize member-display-alt-address-style Created attachment 102481 [details] [review] Bug 25070: Centralize member-main-address-style Created attachment 102487 [details] [review] Bug 25070: Centralize member-alt-address-style There are 5 includes files per value of AddressFormat (us, de, fr). The code is duplicated for each language, the only thing that changes is the order. Wwith this patchset we are going to refactore them to avoid further regressions. The idea is to remove all code that is duplicated and remove the need of having 5*3 files. We are going to limit them to 5 files, 1 per use. Note that we could go even further, for instance member-display-alt-address-style and member-display-address-style are quite identical. Test plan (for the whole patchset): - Create a patron with all the address, contact and alternate info filled - Edit them and save - Look at the patron's info (top left) in the patron module, and the patron detail view. - Play with the different values of AddressFormat The order and the info must be displayed identically before and after this patchset. No change is expected. Created attachment 102488 [details] [review] Bug 25070: Centralize member-alt-contact-style Created attachment 102489 [details] [review] Bug 25070: Centralize member-display-address-style Created attachment 102490 [details] [review] Bug 25070: Centralize member-display-alt-address-style Created attachment 102491 [details] [review] Bug 25070: Centralize member-main-address-style Patch doesn't apply 8-( root@34d7e7da5f3f:koha(bz25070)$ git bz apply 25070 Bug 25070 - Include files to display address and contact must be refactored 102487 - Bug 25070: Centralize member-alt-address-style 102488 - Bug 25070: Centralize member-alt-contact-style 102489 - Bug 25070: Centralize member-display-address-style 102490 - Bug 25070: Centralize member-display-alt-address-style 102491 - Bug 25070: Centralize member-main-address-style Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 25070: Centralize member-alt-address-style Applying: Bug 25070: Centralize member-alt-contact-style Applying: Bug 25070: Centralize member-display-address-style Applying: Bug 25070: Centralize member-display-alt-address-style Applying: Bug 25070: Centralize member-main-address-style Using index info to reconstruct a base tree... M koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-de.inc M koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-fr.inc M koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-us.inc M koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt CONFLICT (modify/delete): koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-us.inc deleted in Bug 25070: Centralize member-main-address-style and modified in HEAD. Version HEAD of koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-us.inc left in tree. CONFLICT (modify/delete): koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-fr.inc deleted in Bug 25070: Centralize member-main-address-style and modified in HEAD. Version HEAD of koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-fr.inc left in tree. CONFLICT (modify/delete): koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-de.inc deleted in Bug 25070: Centralize member-main-address-style and modified in HEAD. Version HEAD of koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-de.inc left in tree. error: Failed to merge in the changes. Patch failed at 0001 Bug 25070: Centralize member-main-address-style Created attachment 104562 [details] [review] Bug 25070: Centralize member-alt-address-style There are 5 includes files per value of AddressFormat (us, de, fr). The code is duplicated for each language, the only thing that changes is the order. Wwith this patchset we are going to refactore them to avoid further regressions. The idea is to remove all code that is duplicated and remove the need of having 5*3 files. We are going to limit them to 5 files, 1 per use. Note that we could go even further, for instance member-display-alt-address-style and member-display-address-style are quite identical. Test plan (for the whole patchset): - Create a patron with all the address, contact and alternate info filled - Edit them and save - Look at the patron's info (top left) in the patron module, and the patron detail view. - Play with the different values of AddressFormat The order and the info must be displayed identically before and after this patchset. No change is expected. Created attachment 104563 [details] [review] Bug 25070: Centralize member-alt-contact-style Created attachment 104564 [details] [review] Bug 25070: Centralize member-display-address-style Created attachment 104565 [details] [review] Bug 25070: Centralize member-display-alt-address-style Created attachment 104566 [details] [review] Bug 25070: Centralize member-main-address-style I'm getting an error when I go to add a new patron - hope this enough of the error trace: Template process failed: file error - member-alt-contact-style-us: not found at /kohadevbox/koha/C4/Templates.pm line 122 in C4::Templates::output at /kohadevbox/koha/C4/Templates.pm line 122 119: 120: my $data; 121: binmode( STDOUT, ":utf8" ); 122: $template->process( $self->filename, $vars, \$data ) 123: || die "Template process failed: ", $template->error(); 124: return $data; 125: } Show function arguments in (eval) at /kohadevbox/koha/members/memberentry.pl line 86 Hum, I reused patches from a local branch, I should have not apparently :-/ Created attachment 104581 [details] [review] Bug 25070: Centralize member-alt-address-style There are 5 includes files per value of AddressFormat (us, de, fr). The code is duplicated for each language, the only thing that changes is the order. Wwith this patchset we are going to refactore them to avoid further regressions. The idea is to remove all code that is duplicated and remove the need of having 5*3 files. We are going to limit them to 5 files, 1 per use. Note that we could go even further, for instance member-display-alt-address-style and member-display-address-style are quite identical. Test plan (for the whole patchset): - Create a patron with all the address, contact and alternate info filled - Edit them and save - Look at the patron's info (top left) in the patron module, and the patron detail view. - Play with the different values of AddressFormat The order and the info must be displayed identically before and after this patchset. No change is expected. Created attachment 104582 [details] [review] Bug 25070: Centralize member-alt-contact-style Created attachment 104583 [details] [review] Bug 25070: Centralize member-display-address-style Created attachment 104584 [details] [review] Bug 25070: Centralize member-display-alt-address-style Created attachment 104585 [details] [review] Bug 25070: Centralize member-main-address-style Sorry David, it should be ok now. (In reply to Jonathan Druart from comment #29) > Sorry David, it should be ok now. Thanks Jonathan - it all works now! Two things I noted: 1) Alternative address section: the email address displays as empty on the patron details page, even though it has a value. 2) Alternative contact section: the address displays with each field listed, rather than as an address block like the contact information and alternative address sections. As both of these reflect the current behaviour, I'm not sure whether you want to fix these in this bug or do separately. Apart from those two points, everything seems to work and I'm happy to sign off. We should fix them on a separate bug report, to make the bug fixes easily backportable. (In reply to David Nind from comment #30) I have fixed: > 1) Alternative address section: the email address displays as empty on the > patron details page, even though it has a value. on bug 25452. I let you open a bug report for the second remark ;) Created attachment 104659 [details] [review] Bug 25070: Centralize member-alt-address-style There are 5 includes files per value of AddressFormat (us, de, fr). The code is duplicated for each language, the only thing that changes is the order. Wwith this patchset we are going to refactore them to avoid further regressions. The idea is to remove all code that is duplicated and remove the need of having 5*3 files. We are going to limit them to 5 files, 1 per use. Note that we could go even further, for instance member-display-alt-address-style and member-display-address-style are quite identical. Test plan (for the whole patchset): - Create a patron with all the address, contact and alternate info filled - Edit them and save - Look at the patron's info (top left) in the patron module, and the patron detail view. - Play with the different values of AddressFormat The order and the info must be displayed identically before and after this patchset. No change is expected. Signed-off-by: David Nind <david@davidnind.com> Created attachment 104660 [details] [review] Bug 25070: Centralize member-alt-contact-style Signed-off-by: David Nind <david@davidnind.com> Created attachment 104661 [details] [review] Bug 25070: Centralize member-display-address-style Signed-off-by: David Nind <david@davidnind.com> Created attachment 104662 [details] [review] Bug 25070: Centralize member-display-alt-address-style Signed-off-by: David Nind <david@davidnind.com> Created attachment 104663 [details] [review] Bug 25070: Centralize member-main-address-style Signed-off-by: David Nind <david@davidnind.com> (In reply to Jonathan Druart from comment #32) > I let you open a bug report for the second remark ;) Bug 25463 opened. Created attachment 105745 [details] [review] Bug 25070: Centralize member-alt-address-style There are 5 includes files per value of AddressFormat (us, de, fr). The code is duplicated for each language, the only thing that changes is the order. Wwith this patchset we are going to refactore them to avoid further regressions. The idea is to remove all code that is duplicated and remove the need of having 5*3 files. We are going to limit them to 5 files, 1 per use. Note that we could go even further, for instance member-display-alt-address-style and member-display-address-style are quite identical. Test plan (for the whole patchset): - Create a patron with all the address, contact and alternate info filled - Edit them and save - Look at the patron's info (top left) in the patron module, and the patron detail view. - Play with the different values of AddressFormat The order and the info must be displayed identically before and after this patchset. No change is expected. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 105746 [details] [review] Bug 25070: Centralize member-alt-contact-style Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 105747 [details] [review] Bug 25070: Centralize member-display-address-style Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 105748 [details] [review] Bug 25070: Centralize member-display-alt-address-style Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 105749 [details] [review] Bug 25070: Centralize member-main-address-style Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 105750 [details] [review] Bug 25070: (QA follow-up) Fix capitalization: Street Number Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to master for 20.11, thanks to everybody involved! backported to 20.05.x for 20.05.01 enhancement, not backported to 19.11.x |