Skip to content
This repository was archived by the owner on Dec 25, 2024. It is now read-only.

Feature History#570

Open
JustyAnOther wants to merge 32 commits intoopacapp:masterfrom
JustyAnOther:FeatureHistory2
Open

Feature History#570
JustyAnOther wants to merge 32 commits intoopacapp:masterfrom
JustyAnOther:FeatureHistory2

Conversation

@JustyAnOther
Copy link
Contributor

Hallo Raphael, hallo Johan,

für Leute (wie mich) die gerne wissen was sie im Laufe der Zeit wann ausgeliehen haben, hier ein Feature-Vorschlag um in Web Opac App eine Chronik aller jemals ausgeliehenen Medien zu führen. Die Implementierung basiert auf dem Code zur Merkliste. Die Inhalte der Database werden automatisch (d. h. ohne Benutzeraktion) durch Abgleich beim Update des Kontos gepflegt. Man kann in der Chronik

  • ein Item löschen oder Details anzeigen lassen
  • nach unterschiedlichen Feldern sortieren
  • im JSON-Format ex- und importieren

(Noch) nicht umgesetzt sind folgende Ideen

  • alle Items auf einmal löschen
  • das Führen der Chronik über eine Option in den Einstellungen auszuschalten
  • die Chronik bibliotheksübergreifend anzuzeigen
  • Share/Teilen der Chronik: Momentan werden nur die Titel ausgegeben.

Schaut euch meinen Vorschlag mal an, ob er von allgemeinem Interesse ist. Bei Bedarf kann ich noch ein paar Screenshots liefern.

Mit Neujahrs-Grüßen - Michael

@raphaelm
Copy link
Member

raphaelm commented Jan 5, 2020

Hi Michael,

wow, da hast du uns aber überrascht! Reife Leistung!

Ohne da jetzt schon tief in den Code einzusteigen haben wir das mal kurz im Team besprochen. Erste Gedanken:

  • Es gab in der Vergangenheit immer wieder Diskussionen über das Feature und starke datenschutzrechtliche Bedenken der Bibliotheken, auch wenn die Daten nur lokale gespeichert werden. Wir würden uns daher nicht nur wünschen, dass das Feature in den Einstellungen abgestellt werden kann – es müsste unserer Meinung nach auch per Default aus sein. Gerne könnte man Nutzer beim Update auf das neue Feature z.B. mit einem einmaligen Dialog hinweisen.

  • Du scheinst die Liste anhand von Titel und Autor zu führen – das ist sehr oft nicht eindeutig, z.B. bei Zeitschriftenbänden. In manchen Systemen kriegen wir im Konto die Medien-ID, leider nicht in allen. Aber wenn wir sie haben, sollten wir sie auch nutzen :)

  • Zweigstellen zu speichern macht in der Historie wohl wenig Sinn, ich kann ja durchaus gleiche oder veschiedene Exemplare des gleichen Mediums in verschiedenen Zweigstellen ausgeliehen haben und das ist für die Historie wohl wenig relevant.

Viele Grüße
Raphael

@JustyAnOther
Copy link
Contributor Author

Hallo Raphael,
danke für's Drüberschauen und die schnelle Reaktion.

  • an Datenschutz habe nicht gedacht, aber das ist berechtigt.
  • beim Abgleich der Ausleihen mit der Historie (Methode HistoryItem.isSameAsLentItem) fehlt der Vergleich auf Medien-Id, das ist richtig. Beim Import (Methode HistoryDataSource.insertOrUpdate) hatte ich es beachtet.
  • ob Zweigstellen in der Historie relevant sind, ist Geschmackssache. Da die Inhalte des HistoryItem aus dem LentItem gepflegt werden, hatte ich alle Felder von dort einfach übernommen. Man hätte ev. das HistoryItem aus dem LentItem ableiten können, aber aus irgendeinem Grund, den ich vergessen habe, habe ich das nicht gemacht

Ich werde in den nächsten Tage versuchen die Vorschläge umzusetzen.
Grüße - Michael

Copy link
Collaborator

@johan12345 johan12345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Michael,

ob Zweigstellen in der Historie relevant sind, ist Geschmackssache. Da die Inhalte des HistoryItem aus dem LentItem gepflegt werden, hatte ich alle Felder von dort einfach übernommen. Man hätte ev. das HistoryItem aus dem LentItem ableiten können, aber aus irgendeinem Grund, den ich vergessen habe, habe ich das nicht gemacht

Das gilt hier nicht nur für die Zweigstellen, sondern auch für das Feld status eines AccountItem. Das macht bei einem HistoryItem nicht wirklich Sinn, denn der Status ist nicht mehr korrekt, oder zumindest nicht mehr relevant, wenn man das Buch zurückgegeben hat. Darum würde ich es in der Detailansicht eines HistoryItems nicht anzeigen.

Bei den Zweigstellen würde ich auch sagen, die zu speichern (bzw. anzuzeigen) macht nur Sinn, solange man jedes Buch nur einmal ausgeliehen hat. Sobald man etwas mehrfach ausleicht, was ja durchaus passieren kann, wird nur noch der letzte Wert gespeichert, oder? Selbst wenn man immer in der gleichen Zweigstelle ausleiht, kann es ja sein, dass das Buch von einer anderen Zweigstelle bestellt wurde (lendingBranch unterscheidet sich dann). Wenn man für jeden Entleihvorgang des selben Buchs die Zweigstellen speichert, wird die Datenstruktur wieder komplizierter - vielleicht ist es dann besser, sie wegzulassen...

Ich habe außerdem mal ein bisschen weiter getestet und noch folgende Hinweise:

  • Die Strings "Beginndatum" und "Endedatum" (bzw. "Start date" und "Last date") sind nicht mehr so sinnvoll, wenn man das gleiche Medium mehrfach ausgeliehen hat. Ich habe Alternativvorschläge jeweils dran geschrieben.
  • Auch die Anzeige der Leihdauer ("for ... days"), die aus Start- und Enddatum berechnet wird, stimmt wahrscheinlich nicht mehr, wenn man etwas mehrfach ausgeliehen hat (das kann ja mit einem langen Zeitabstand passieren). Hast du eine Idee, wie man das ohne viel Komplexität verbessern kann? Ansonsten sollten wir vielleicht einfach die beiden Daten anzeigen statt Startdatum und Anzahl Tage.
  • Du hast die Aktualisierung der Historie momentan nur im AccountFragment implementiert. Es würde aber Sinn machen, die auch bei der Synchronisierung der Kontodaten im Hintergrund mit zu aktualisieren, das passiert in de.geeksfactory.opacclient.reminder.SyncAccountJob. Der SyncAccountJob muss dann auch prüfen, ob die Historiefunktion aktiviert ist, um dann zu entscheiden, ob die Kontodaten synchronisiert werden müssen oder nicht (so, wie es das jetzt schon für die Benachrichtigungs-Funktion macht).
  • Wenn die Merkliste ein Medium enthält und ich dann von der Merkliste zur Historie wechsle, stürzt die App bei mir ab:
java.lang.IllegalArgumentException: Invalid format: "Heilbronn"
        at org.joda.time.format.DateTimeFormatter.parseLocalDateTime(DateTimeFormatter.java:900)
        at org.joda.time.format.DateTimeFormatter.parseLocalDate(DateTimeFormatter.java:844)
        at org.joda.time.LocalDate.parse(LocalDate.java:179)
        at org.joda.time.LocalDate.parse(LocalDate.java:168)
        at de.geeksfactory.opacclient.storage.HistoryDataSource.cursorToItem(HistoryDataSource.java:255)
        at de.geeksfactory.opacclient.frontend.HistoryFragment$ItemListAdapter.bindView(HistoryFragment.java:612)
        at androidx.cursoradapter.widget.CursorAdapter.getView(CursorAdapter.java:274)

Werden da irgendwie Datenbanktabellen verwechselt?

  • Ist es vielleicht möglich, bei der Sortierfunktion noch eine Anzeige einzubauen, wonach momentan sortiert wird? Einfach zu dem jeweiligen Menüpunkt ein Symbol hinzufügen, das die Sortierrichtung anzeigt?
  • Im Gegensatz zu Suchergebnissen (die in der Merkliste gespeichert werden) haben wir für entliehene Medien in sehr viel weniger Bibliothekssystemen die Möglichkeit, Medientyp und/oder Cover herauszufinden. Kannst du vielleicht in dem Fall, dass kein Element der Historie ein Cover oder einen Medientyp hat, die beiden Felder ausblenden und damit mehr Platz für den Titel und Autor schaffen?
  • Toll wäre später noch eine Suchfunktion, die Historie kann ja sehr lang werden. Aber das muss nicht in diesem PR passieren.
  • Bitte die Funktion "Reformat Code" von Android Studio benutzen, um den Code einheitlich zu formatieren. Zum Beispiel fehlen manchmal Leerzeichen links und rechts von =.

<string name="star_wait">Please wait for the details to load.</string>
<string name="star_unsupported">We could not obtain enough information about this item to add it to your list.</string>
<string name="starred_welcome">This is your favorites list. You can add items by clicking the star on a detail page. The list is saved separately for every library you use.</string>
<string name="history_welcome">This is your history list. You can The list is saved separately for every library you use.</string>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unnecessary "You can" should probably be removed.

Maybe it makes sense to add some more explanation, e.g.: "This is lending history. It keeps a list of all items you have lent in the past. The list is saved separately for every library you use."

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ups, das "your" aus "This is is your lending history" kann bleiben. Sorry!

<string name="nav_history">Chronik</string>
<string name="share_history">Teilen</string>
<string name="export_history_to_storage">Exportieren</string>
<string name="history_welcome">Dies ist deine Leih-Chronik. Die Liste wird für jede Bibliothek, die du benutzt, separat geführt.</string>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see below, maybe add something like "Hier werden alle Medien gespeichert, die du in der Vergangenheit entliehen hast."

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Dies ist deine Leih-Chronik" kann davor auch bleiben

@johan12345
Copy link
Collaborator

Share/Teilen der Chronik: Momentan werden nur die Titel ausgegeben.

Macht so eine Teilen-Funktion der gesamten Historie überhaupt Sinn? Bei ausgiebiger Bibliotheksnutzung können da ja irgendwann hunderte Bücher drin stehen, die will man wahrscheinlich nicht mal so eben als endlos lange Textnachricht per Messenger versenden... 😉 Wenn, dann würde ich die eher als PDF versenden oder ausdrucken.

@JustyAnOther
Copy link
Contributor Author

Eine Klarstellung was meine Idee der History betrifft. Auf Grund euerer Fragen habe ich gemerkt das ihr das anders verstanden habt.
Die History soll eine Liste aller Ausleihungen sein,

  • d. h. wenn ein Medium 2 Mal ausgeliehen wird, taucht es auch 2 Mal in der History auf.
  • das vereinfacht die Pflege der Tabelle.
  • der Abgleich der Ausleihen findet nicht gegen die gesamte History statt, sondern nur gegen die Ausleihen, die beim vorherigen Abgleich laufend waren (lending = true, quasi den vorherigen Kontostand). Das findet sich nicht im JAVA-Code sondern in der verwendeten SQL-Bedingung HistoryDatabase.HIST_WHERE_LIB_LENDING = "bib = ? AND lending = 1";. Wenn lending mal auf false gesetzt ist, wird der History-Satz nicht mehr verändert.
  • der Abgleich dürfte dadurch auch performanter sein
  • dadurch hat man ein einfache Tabelle, keine Normalisierung, trotzdem alle Informationen zur Ausleihe.
  • Gegegenfalls kann durch geeignete SQL-Statement mit group-by mehrfache Ausleihen eines Mediums erkannt und angzeigt werden.

Ich denke das ist sinnvoll und vermeidet einige Probleme die ihr mir eueren Fragen angesprochen habt.

HistoryFragment: initLoader mit id=1 (!=0 wie bei StarFragment) wg.
 Datenbankverwechslung bei Wechsel Favorits --> History
@JustyAnOther
Copy link
Contributor Author

Folgende Punkte habe ich mit den heute gepushten Commits umgesetzt:

  • Löschen der History zum aktuellen Account möglich (mit Are you sure? -Rückfrage)
  • History nur optional führen via Preference (d. h. nur für alle Account einheitlich möglich)
  • Wird das Führen der History deaktiviert, so ist es möglich die History (zu allen Accounts) nach Rückfrage zu löschen.
  • einige history_strings korrigiert entsprechend eueren Vorschlägen
  • HistoryItem nach opacapp/storage verschoben
  • Exception beim Wechsel von Merkliste zu History behoben
  • Reformat code

Offen ist zumindest noch

  • status bei Detail-Anzeige von HistoryItem ist nicht sinnvoll
  • Share/Teilen: Was ist sinnvoll? PDF oder CSV um es in Excel zu importieren? Wobei manches für mich weniger mit Teilen als mit Export in einem alternativen Format zu tun hat.
  • Wir wird mit dem Menuitem History im navigation_drawer umgegangen, wenn History deaktiviert ist? Hier habe ich noch nicht herausgefunden, wie das Menuitem versteckt oder deaktiviert werden kann.

@johan12345
Copy link
Collaborator

johan12345 commented Jan 14, 2020

Eine Klarstellung was meine Idee der History betrifft. Auf Grund euerer Fragen habe ich gemerkt das ihr das anders verstanden habt. Die History soll eine Liste aller Ausleihungen sein.

Ah, okay, das macht Sinn. Ich hatte zuerst intuitiv gedacht, dass es um eine Liste aller jemals ausgeliehenen Medien (d.h. ohne Dopplung bei mehrfacher Ausleihe) geht - unter anderem, weil so eine Funktion häufiger bei uns angefragt wurde. Aber die Liste aller Ausleihungen ist auch okay, häufig ist die ja identisch bzw. die Liste der ausgeliehenen Medien lässt sich leicht daraus ableiten.

Dann machen natürlich einige meiner Kommentare zu den Strings oben keinen Sinn mehr, ich passe das mal an.

Wir wird mit dem Menuitem History im navigation_drawer umgegangen, wenn History deaktiviert ist? Hier habe ich noch nicht herausgefunden, wie das Menuitem versteckt oder deaktiviert werden kann.

Das geht wohl mit

drawer.getMenu().findItem(R.id.nav_history).setVisible(false) // bzw.
drawer.getMenu().findItem(R.id.nav_history).setEnabled(false)

Ich weiß gerade nicht, welche der beiden Optionen mir lieber ist - zumindest müsste man aber im Fall, dass es nur deaktiviert ist, irgendwie erkennen können, wie es aktiviert werden kann.

Share/Teilen: Was ist sinnvoll? PDF oder CSV um es in Excel zu importieren? Wobei manches für mich weniger mit Teilen als mit Export in einem alternativen Format zu tun hat.

Bin ich mir gerade auch unsicher. @raphaelm, was meinst du? PDF könnte man über die Druckfunktion von Android lösen (so wie man schon ein Suchergebnis drucken kann, siehe SearchResultDetailFragment.print()). Aber wir können das auch erstmal raus lassen und in einem separaten PR machen.

History Liste: Save/Restore Sortierung aus InstanceState
Details: bei History status nicht anzeigen; prolongCount ergänzt
NPE bei UpdateHeader ohne Context vermeiden
@JustyAnOther
Copy link
Contributor Author

Hallo Johan,
ich habe jetzt eine Stand erreicht der meines Erachtens ganz gut aussieht

  • Sortierung wird als Subheader angezeigt und in SharedPrefs und Bundle-State gesichert und wiederhergestellt
  • Abgleich in SyncAccoutJob eingebunden (hier scheint es aber einen Konflikt mit euren Änderung zu geben)
  • Share habe ich als CSV umgesetzt, aber im Menu weiter nach hinten verschoben
  • Anzeige von Cover (konnte ich nicht testen) und MediaType-Spalte nur falls vorhanden
  • AccountItem.status aus DB-Tabelle entfernt, da es bei History keinen Sinn macht.
    Ich werde den Stand mal eine Weile bei mir benützen, um zu sehen ob es noch Bugs oder Unstimmigkeiten gibt. Falls ihr noch Vorschläge habt, bitte melden.
    Grüße - Michael

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants