Project

General

Profile

Bug #3078

Misvisende meddelse ved fornyelse af flere lån. (AFVENTER #3817; TESTES NÅR DEN ER LØST)

Added by Mathias Larsen over 2 years ago. Updated almost 2 years ago.

Status:
Resolved (tag version)
Priority:
Urgent
Assignee:
Estimated time:
URL med eksempel:
Kategorier:
Min konto - Lån

Description

Ved fornyelse af flere lån, vises en besked med at materialet er fornyet. En for hvert materiale. 

Titlen er dog altid fra den sidste af de materiale, som man vil fornye (det sidste materiale med et flueben).

Dette sker kun, hvis der ikke er noget data fra OpenSearch, men titlen kommer direkte fra provideren, hvilket er fallback, hvis OpenSearch ikke har noget.

Screen Shot 2017-11-20 at 16.01.04.png (16.7 KB) Screen Shot 2017-11-20 at 16.01.04.png Besked for de to materialer, der bliver fornyet. Mathias Larsen, 11/20/2017 04:07 PM
Screen Shot 2017-11-20 at 16.02.18.png (87.1 KB) Screen Shot 2017-11-20 at 16.02.18.png Valgte materialer til fornyelse. Mathias Larsen, 11/20/2017 04:07 PM
Skærmbillede 2018-09-19 15.20.19.png (15.3 KB) Skærmbillede 2018-09-19 15.20.19.png Første gang jeg fornyer flere materialer får jeg denne side i stedet for en bekræftelse Gitte Barlach, 09/19/2018 03:47 PM

Related issues

Related to DDB CMS - Bug #3747: Forny lån - Forvirrende besked til låner Closed
Related to DDB CMS - Bug #3841: Customerror modulet - uhensigtsmæssige redirectsResolved (tag version)
Related to DDB CMS - Bug #3817: Fejlagtige / forstyrrende ding_entity_rating/get? kald Resolved (tag version)
Has duplicate DDB CMS - Bug #2595: Fjernlån: Forkert lånerbesked på fjernlån, som bliver fornyet (Titlen er ikke tilgængelig er fornyet)Closed

History

#2 Updated by Rolf Madsen over 2 years ago

  • Status changed from New to Needs code review
  • Assignee set to Gitte Barlach
  • Priority changed from Normal to Urgent
  • Target version set to Release 27 - Bugfixes (2017 2. opgradering) (7.x-4.2.1)

Tak for PR Mathias!

Jeg tror det er samme problem vi har fået meldt ind omkring fjernlån.

Det skaber usikkerhed for brugere der fornyer materialer, hvilket ikke er hensigtsmæssigt.

#3 Updated by Gitte Barlach over 2 years ago

  • Assignee changed from Gitte Barlach to Kasper Garnæs

#4 Updated by Simon Holt over 2 years ago

Inden vi får merget/kigget på den her, synes jeg lige vi skal have merget PR fra denne her sag: #2564

Her har jeg arbejdet med at få den til at vise alle titler, hvis der er flere materialer på et mellemværende og har desuden også fikset nogle situationer, hvor den ikke viser titel.

PR er blevet godkendt og mangler bare merge. Jeg har ændret en del i den samme kode og så gerne vi mergede mit PR først, så vi kan se hvordan det spiller sammen med den foreslåede løsning i denne sag.

 

Et par kommentarer til PR i denne sag:

Mht. til titel fra provideren: Mener bestemt at FBS kun vidste noget om titlen ved fjernlån, men ved almindelige lån af f.eks. lokal- og påhængsposter vidste den ikke noget og her kan den ofte heller ikke finde titlen ved søgning i brønden.

ding_provider_get_pseudo_entity() har i hvert fald ikke meget effekt, da populate_pseudo_entity() ikke implementeres i FBS (jvf. fbs.loan.inc og alma.loan.inc). Så den sætte aldrig nogen titel.

 

#5 Updated by Mathias Larsen over 2 years ago

Mit PR berører kun lån, hvor dit har med debts at gøre.

Derfor burde de ikke konflikte, da vi ikke har med de samme filer eller kald at gøre.

Simon Holt wrote:

Inden vi får merget/kigget på den her, synes jeg lige vi skal have merget PR fra denne her sag: #2564

Her har jeg arbejdet med at få den til at vise alle titler, hvis der er flere materialer på et mellemværende og har desuden også fikset nogle situationer, hvor den ikke viser titel.

PR er blevet godkendt og mangler bare merge. Jeg har ændret en del i den samme kode og så gerne vi mergede mit PR først, så vi kan se hvordan det spiller sammen med den foreslåede løsning i denne sag.

 

Et par kommentarer til PR i denne sag:

Mht. til titel fra provideren: Mener bestemt at FBS kun vidste noget om titlen ved fjernlån, men ved almindelige lån af f.eks. lokal- og påhængsposter vidste den ikke noget og her kan den ofte heller ikke finde titlen ved søgning i brønden.

ding_provider_get_pseudo_entity() har i hvert fald ikke meget effekt, da populate_pseudo_entity() ikke implementeres i FBS (jvf. fbs.loan.inc og alma.loan.inc). Så den sætte aldrig nogen titel.

 

 

#6 Updated by Kasper Garnæs over 2 years ago

  • Assignee changed from Kasper Garnæs to Gitte Barlach

Gitte: Pga. vores guidelijes kan jeg ikke reviewe denne. Mathias er min kollega. Kan du finde en anden til at foretage review?

Derudover er jeg enig i hans pointe. Dette PR vil ikke konflikte med #2564, da de berører forskellige områder - henholdsvis lån og reservationer.

#7 Updated by Simon Holt over 2 years ago

Mit PR berører kun lån, hvor dit har med debts at gøre.

Beklager, det er ganske rigtig. Havde fået blandet tingene sammen.

#8 Updated by Gitte Barlach over 2 years ago

  • Assignee changed from Gitte Barlach to Jørgen Nielsen

#9 Updated by Mathias Larsen over 2 years ago

Simon Holt wrote:

Mit PR berører kun lån, hvor dit har med debts at gøre.

Beklager, det er ganske rigtig. Havde fået blandet tingene sammen.

No worries. :) 

#10 Updated by Jørgen Nielsen over 2 years ago

  • Status changed from Needs code review to Reviewed
  • Assignee changed from Jørgen Nielsen to Gitte Barlach


reviewet og godkendt

#11 Updated by Kasper Garnæs over 2 years ago

  • Status changed from Reviewed to Technical test

Merged.

#12 Updated by Michael W. Christoffersen over 2 years ago

@Gitte - Rolf skriver: "Som jeg læser det skal vi finde et materialer som biblioteket ikke har i beholdning, f. eks. et fjernlån." Kan du teste det?

#13 Updated by Kasper Garnæs over 2 years ago

Dette ticket er blevet trukket ud af release 27 pga. manglende mulighed for test. De kodemæssige ændringer er rullet tilbage: https://github.com/ding2/ding2/pull/994.

#14 Updated by Rolf Madsen over 2 years ago

  • Target version changed from Release 27 - Bugfixes (2017 2. opgradering) (7.x-4.2.1) to Release 29-2 - Bugfixes (Reload)

#15 Updated by Gitte Barlach over 2 years ago

  • Status changed from Technical test to Reviewed

#16 Updated by Gitte Barlach over 2 years ago

  • Status changed from Reviewed to Development

#17 Updated by Rolf Madsen over 2 years ago

  • Description updated (diff)

#18 Updated by Rolf Madsen over 2 years ago

  • Has duplicate Bug #2595: Fjernlån: Forkert lånerbesked på fjernlån, som bliver fornyet (Titlen er ikke tilgængelig er fornyet) added

#19 Updated by Gitte Barlach over 2 years ago

  • Status changed from Development to Reviewed - Needs info/rework
  • Assignee changed from Gitte Barlach to Mathias Larsen

#20 Updated by Gitte Barlach over 2 years ago

  • Status changed from Reviewed - Needs info/rework to Reviewed
  • Assignee changed from Mathias Larsen to Gitte Barlach

#21 Updated by Kasper Garnæs almost 2 years ago

  • Status changed from Reviewed to Ready for development

Denne ændring er blevet trukket tilbage. Den skal genintroduceres - fx. ved at reverte det PR der revertede ændringen oprindeligt.

#22 Updated by Rolf Madsen almost 2 years ago

  • Status changed from Ready for development to Reviewed - Needs info/rework
  • Assignee changed from Gitte Barlach to Christel Krabbenhøft

Jeg har spurgt Lotte om hun kan lave et fjernlån når vi har merged og udrullet ændringen.

Derfor sætter jeg status til Reviewed - needs rework med henblik på revert af PR.

Assigend til Christel med henblik på allokering af Mathias.

#23 Updated by Rolf Madsen almost 2 years ago

  • Subject changed from Misvisende meddelse ved fornyelse af flere lån. to Misvisende meddelse ved fornyelse af flere lån.BED LOTTE OM FJERNLÅN NÅR TEST NÆRMER SIG

Lotte vil gerne hjælpe, men skal lige have en advis når vi ved hvornår opgaven kan løses.

#24 Updated by Christel Krabbenhøft almost 2 years ago

Reload har bekræftet, at Mathias kan kigge på den i næste uge - Lotte, har du behov for at vide præcis hvornår i næste uge?

#25 Updated by Kasper Garnæs almost 2 years ago

Som jeg ser det er det ikke først relevant at have testdata klar, når rettelsen er blevet en del af en release som er klar til test.

Pt. er opgaven at få genintroduceret ændringen så den kan blive en del af releasen.

#26 Updated by Mathias Larsen almost 2 years ago

  • Status changed from Reviewed - Needs info/rework to Needs code review
  • Assignee changed from Christel Krabbenhøft to Kasper Garnæs

#27 Updated by Kasper Garnæs almost 2 years ago

  • Status changed from Needs code review to Technical test
  • Assignee changed from Kasper Garnæs to Gitte Barlach

Merged.

#28 Updated by Rolf Madsen almost 2 years ago

  • Related to Bug #3747: Forny lån - Forvirrende besked til låner added

#29 Updated by Gitte Barlach almost 2 years ago

  • Subject changed from Misvisende meddelse ved fornyelse af flere lån.BED LOTTE OM FJERNLÅN NÅR TEST NÆRMER SIG to Misvisende meddelse ved fornyelse af flere lån. (AFVENTER #3817; TESTES NÅR DEN ER LØST)

#30 Updated by Gitte Barlach almost 2 years ago

Jeg prøver at teste denne!

#31 Updated by Gitte Barlach almost 2 years ago

Testst på upgrade-29 med 4.5.0-rc3

Jeg logger ind med testbruger Emil Birkely. Umiddelbart efter indlogning fornyer jeg to lån: 1 alm. lån og 1 fjernlån 

Af uransaglige årsager kommer jeg til denne helt tomme side første gang jeg klikker på "forny valgte":

https://upgrade-29.ddbcms.dk/autologout_ahah_logout

 

Selvom jeg senere har forsøgt at genskabe fejlen udne helt (det er begrænset hvad jeg har af lån på brugeren og der er andre forny-sager jeg skal teste) så er det ikke umiddelbart lykkedes. Jeg kan dog ikke set bort fra at der opstod en fejl, så jeg kan ikke godkende denne. 

#32 Updated by Simon Holt almost 2 years ago

Se min kommentar her: https://platform.dandigbib.org/issues/3817#note-26

Det kan muligvis være den problematik jeg nævner med DDB CMS og customerror, der driller her igen.

#33 Updated by Rolf Madsen almost 2 years ago

  • Related to Bug #3841: Customerror modulet - uhensigtsmæssige redirects added

#34 Updated by Rolf Madsen almost 2 years ago

  • Related to Bug #3817: Fejlagtige / forstyrrende ding_entity_rating/get? kald added

#35 Updated by Rolf Madsen almost 2 years ago

#3841 er oprettet mhp. at behandle problemet.

#36 Updated by Gitte Barlach almost 2 years ago

  • Status changed from Reviewed - Needs info/rework to Resolved (tag version)

Den grundlæggende problematik
"Ved fornyelse af flere lån, vises en besked med at materialet er fornyet. En for hvert materiale. Titlen er dog altid fra den sidste af de materiale, som man vil fornye (det sidste materiale med et flueben). Dette sker kun, hvis der ikke er noget data fra OpenSearch, men titlen kommer direkte fra provideren, hvilket er fallback, hvis OpenSearch ikke har noget." har jeg testet via fornyelse af fjernlån. Dette ser ud til at fungere som det skal.

Den fejl der er nævnt i kommentar 31, er løst med #3841

Jeg sætter derfor denne til Resolved (tag version)

Also available in: Atom PDF