[FINNA-2767] Initial implementation of disec support for reservation lists #3091
Conversation
module/Finna/src/Finna/Controller/ReservationListController.php
Outdated
Show resolved
Hide resolved
module/Finna/src/Finna/ReservationList/ReservationListStatus.php
Outdated
Show resolved
Hide resolved
module/Finna/src/Finna/ReservationList/ReservationListStatus.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
- Status-teksteistä vielä sen verran, että jos käyttäjän pitäisi niiden perusteella osata jotenkin toimia tai olla toimimatta, niin ei ole välttämättä mitenkään liian selkeä. Tarkoittaako esim. "processed", että aineisto on matkalla käyttäjälle, vai jotain muuta? Entä "Aineisto poistettu"?
- Laita kaikkiin
PHP version 8.1=>PHP version 8. Niin se on muuallakin. - Kaikki Connection:iin liittyvä pitäisi olla vielä omassa nimiavaruudessa, esim.
Finna\ReservationList\Connection.
| # and is the default value, if omitted | ||
| # Connection holds information about type of lists: | ||
| # feedbackform: Default connection, orders handled with sending forms | ||
| # handler: email [Optional] Default handler email, other values found in FeedbackForms.yaml primary handlers |
There was a problem hiding this comment.
Miten tämä suhteutuu FeedbackForms.yaml:n asetuksiin? Jos tähän laittaa vaikka api, niin mitä asetuksia se käyttää? Olisiko parempi jättää lähetystapa formin puolella määriteltäväksi niin, että lähetys feedbackforms:n kautta käyttää oikeasti lomaketta eikä vain handleria? Muussa tapauksessa olisi parempi irrottaa handlerit sieltä ReservationList:n connectioneiksi.
| 'LibraryCardSources' => [], | ||
| 'CheckResourceStatus' => false, | ||
| 'Connection' => [ | ||
| 'type' => 'Database', |
| ): void { | ||
| if (!$this->userCanEditList($user, $list)) { | ||
| throw new ListPermissionException('list_access_denied'); | ||
| } | ||
| $list->setPickupDate(DateTime::createFromFormat('Y-m-d', $request->get('pickup_date')))->setOrdered(); | ||
| $list->setPickupDate(DateTime::createFromFormat('Y-m-d', $newValues['pickup_date']))->setOrdered(); | ||
| if ($externalId = $newValues['external_id'] ?? null) { |
There was a problem hiding this comment.
Voisiko setExternalId ottaa null:n, kun getExternalId voi palauttaa sen? Silloin riittäisi $list->setExternalId($newValues['external_id'] ?? null); joka varmistaisi, ettei jää väärää arvoa. Vähän sama setConnection:n kohdalla, paitsi että pitäisi laittaa tyhjäksi tai oletukseksi, jos ei muuta ole.
| 'pickup_date' => $postRequest->get('pickup_date'), | ||
| 'resourceIDs' => $postRequest->get('resourceIDs'), | ||
| ]; | ||
| if ($handler instanceof \Finna\ReservationList\FeedbackForm) { |
There was a problem hiding this comment.
Tämä ero kutsuissa pitää saada pois, nyt on aika sotkuinen.
| 'resourceIDs' => $postRequest->get('resourceIDs'), | ||
| ]; | ||
| if ($handler instanceof \Finna\ReservationList\FeedbackForm) { | ||
| $result = $handler->placeOrder($this->params(), $user, $form); |
There was a problem hiding this comment.
Miksi $this->params() eikä $postRequest?
| $connectionType = $listProperties['Connection']['type']; | ||
| $handler = $this->getService(\Finna\ReservationList\PluginManager::class)->get($connectionType); | ||
| $handler->init($listProperties); | ||
| $gatheredPostValues = [ |
There was a problem hiding this comment.
Tämän nimi voisi olla vaikka $listValues tms. kun siihen alempana lisätään muutakin kuin kerättyjä POST-kenttiä.
ALTER TABLE finna_resource_list ADD COLUMN external_id varchar(255) DEFAULT NULL;ALTER TABLE finna_resource_list MODIFY connection varchar(40) NOT NULL DEFAULT 'feedbackform';