https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32607 --- Comment #14 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Agustín Moyano from comment #13) A global response would be (and sorry if I am rambling here) that we enforce consistency. We must not have 2 ways to do the same thing. If you want to add something new to the Vue code in Koha, it must be done globally. Otherwise we end up with divergences. We cannot allow a module that would do something differently.
3. (naming) It's not clear what is Koha::ImportSource->patron and import_source.patron_id, could we find something more meaningful?
It is the patron account that is allowed to create records with this source.. I'm not sure what could we call it.. Koha::RecordSource->allowed_patron and record_sources.allowed_patron_id?
I am bad at naming, keep patron_id for now. (Is this correct to have 1-N instead of N-N then?)
7. FormKit/Form => You are introducing a change that is not applied to other Vue files, and you are changing the UI, it's not consistent with other area of Koha. I don't think we are ready for that.
I believe Formkit or another form library it's a good idea, and we are in a good time, in that we are at the very beginning of using Vue. I believe it can help a lot with the maintainability of the components. For example, if we apply a library that creates forms from a yaml or json definition, theoretically we could be able to reduce ERM components to 4: a Main component, a List component, a Form component and a Show component, and what get's displayed and the actions could be just parameters that are passed or picked up by those components.
If your concern is that it's not implemented in other vue files, we can make other bugs to implement them. But if it is about the UI, css classes can always be modified.
Yes, of course it would be nice to have such mechanisms to ease writing of our form. But as said previously this new code should not introduce something new it would be the only one to use. First we adapt our code, then we use it in new module. In your position it would be easier to stick to the exist patterns.
8. In ISEdit.vue, processSubmit. The way we are displaying the message is in then(). Same for the other AJAX requests in other components.
It is implemented in the "then" part.. you see, in Javascript if you are in an async function doing
Yes ofc, I know. But still the same answer, it's a different pattern...
10. the routes: you are deciding to change the URL, admin/import-sources.pl/add We are not having the '.pl' for other vue pages (which require the apache's rewriterule, but I think we need to stay consistent)
I forgot about this part, and am not sure how to implement this, but definitely will take a look
See debian/templates/apache-shared-intranet.conf:RewriteRule ^/cgi-bin/koha/erm/.*$ /cgi-bin/koha/erm/erm.pl [PT]
11. import-sources.js You are using add/remove/update/getOne when we use create/delete/update/get in other fetch files.
I cannot use create/delete/update/get because http-client implement those same methods.. one way would be to add a new level like
return { record_source: { get: () .... create: () ... update: () ... delete: () ... } }
but it's kind of redundant since I don't have many things to fetch as in ERM.. and I would end up calling the actions like
record_source.record_source.get()
Yes, that's not a problem, we have it for patron as well, see components/ERM/UserRoles.vue const client = APIClient.patron // FIXME We are missing a "loading..." client.patrons.get(selected_patron_id).then(p => { We can imagine to have something related to patron but that is not "patrons" (like the attributes for instance). Stick to the existing patterns please ;)
The other way would be to rename the methods in http-client to _create/_delete/_update/_get and there would be no collision in the names
12. In patron-api-client.js you are using "list" when we use "getAll" in ERM
Ups, I did not realize that... "list" seems to me as a more intuitive name for it, but have absolutely no problem in changing the name
Yes I agree, we could rethink the verbs. But that's for another bug.
13. About autocomplete, I would not use it. We already have a way to select patrons (in all the different Koha modules, and the same that we reuse in ERM). If we want an autocomplete feature I think we should then use select2 and implement bug 32474.
I did not know about that way to select patrons, but it involves using a popup window. I would not recommend that, as it may get blocked by the browser. I will definitely check select2, but as we are using Vue, I would try to minimize as much as possible the use of jQuery and jQuery components.
Yes, we discussed that recently on #koha. We should have an autocomplete on select2 (with infinite scrolling) to search for patrons. And a "more options" link that would open the new window (that should be a modal). But that is for all the different patron searches in Koha, and so that's a big change, out of the scope here. You should stick to the existing patterns.
I'd like to add that, as usual, I am not against new ways of doing things, but we need to apply them first to the existing code, and be consistent. I have no problem implementing new features to the existing code, but I think it better to be done in another bug so things don't get messed up
Yes, that's all my point! :D -- You are receiving this mail because: You are watching all bug changes.