Resource acquisition: multiple ordering in Coral #244
Conversation
Adding search fields for Publisher, Platform, and Provider.
PHP message: PHP Notice: A session had already been started - ignoring session_start() in /var/www/coral/organizations/user.php on line 40 PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81 PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81 PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81 PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81 PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81 PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81
PHP Notice: Undefined offset: 0 in /var/www/coral/organizations/admin/classes/common/DatabaseObject.php on line 87
Fix purchasing sites on order creation/cloning.
- Split fund name, fund code, payment amount, currency code and order type in multiple rows - Add missing fields: price tax excluded, tax rate, price tax included and order details - Export as many lines as there are acquisition information - Fix broken isbn or issn export
|
Fixed DOS line ending and trailing spaces for the diff to be easier to read. |
|
My environment:
I started testing this. Some notes…
I got past the fatal error by changing line 724 in …/resources/admin/classes/domain/Resource.php to:
See https://github.com/Coral-erm/Coral/blob/development/resources/admin/classes/domain/GeneralSubject.php#L84 for the I added The rest of these notices happened while working in the UI experimenting with adding orders.
In the UI “There was a problem with the database: Field ‘subscriptionStartDate’ doesn’t have a default value” was also displayed when no date was entered. After entering a date and clicking Submit I got the same PHP Notice. I then had to check the Enable Alert box for the window to go away. At that point I had saved two copies of the new order. It seems that the first one saved but the modal window didn’t go away. When trying to Clone the modal does not ever go away, even after checking the Enable Alert box, and additionally I get the following message in the UI:
When editing an order, I have to check Enable Alert as well, but the modal does go away after doing that.
|
|
Also, what’s the plan for users to run the |
|
In showing these potential changes to my colleagues, @lnarizny and @caltechheather, we thought that when changing fields and data structure of the Resources table(s) we really should be sure to update the code for the Importer as well. If these changes go in, users will not be able to import multiple orders through the importer. |
|
Not sure we should do that at this stage given this new feature is optional. This is a big change, we’ve got some needs to iron out the importer as it stands anyway, and I’d rather wait to see how this works.
Steve
On Aug 10, 2017, at 2:00 PM, Tommy Keswick <notifications@github.com<mailto:notifications@github.com>> wrote:
In showing these potential changes to my colleagues, @lnarizny<https://github.com/lnarizny> and @caltechheather<https://github.com/caltechheather>, we thought that when changing fields and data structure of the Resources table(s) we really should be sure to update the code for the Importer as well. If these changes go in, users will not be able to import multiple orders through the importer.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#244 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AC9mt910DInRcRrtaKM_XHxBIB0Kzz-5ks5sW1NNgaJpZM4OA_gh>.
|
|
@t4k Thanks for your comments ! I made a QA fix commit (I couldn't reproduce the clone error, however, and the option to delete an order has yet to be done) I totally agree that the modifications should be part of the new installer, but I'm not familiar with it. Is there somewhere a documentation or a how-to that might help me understand how to add new modifications to it ? |
|
And about the Import tool, it currently creates a default order when importing resources. There are a lot of things the import tool does not handle right now, and I'm not sure importing orders would be the most needed feature to add to it. |
|
@t4k done ! |
|
I've tested this again and got a few of the same problems as the first time:
After entering a date and clicking Submit I get that last PHP Notice. I then had to check the Enable Alert box for the window to go away. At that point I had saved two copies of the new order. It seems that the first one saved but the modal window didn’t go away. When trying to Clone the modal does not ever go away, even after checking the Enable Alert box, and additionally I get the following message in the UI:
When editing an order, I have to check Enable Alert as well, but the modal does go away after doing that. |
|
Update to the comment above. The modal issues arise because on my test environment I have PHP Error Reporting turned on, and this interferes with the modal window behavior because notices are printed all over the screen. I have tested now with PHP Error Reporting turned off and the only UI issue that remains is the message when trying to clone an order record:
A cloned record is created, but the modal does not close. Clicking Cancel and then refreshing shows the cloned record in the select list. (Note, I am using MySQL 5.7 in this test environment.) All that said, is there a way to harden the code so that it can function even if errors are printed on the screen? I really think we should do this, because it seems very fragile otherwise. And also, the underlying PHP Notices still need to be addressed. Turning off Error Reporting is not a solution. |
|
I've added a small patch that prevents the PHP Notices. |
|
More on the clone issue: If I clone an order record, and get that database error, and then cancel out of the modal, there is a brand new cloned resource on the resources list. I don't think this is the intention. |
Fix note edition Fix typos Remove PHP Notices
|
The clone issue should be fixed. |
|
The SQL error is fixed with the latest updated. The issue from my last comment still remains. I have looked further at it and can report more details. I believe the issue is with the query that generates the index page list of resources (/resources/index.php). When there is a resource that has multiple orders, that resource is duplicated in the index list, one for each order record in the system. Maybe there has to be a DISTINCT put on that query or something similar? Steps to reproduce in the sandbox… http://user01-coral-sandbox.test.biblibre.eu/sandbox.php
|
|
fixed. |
|
This will not work for MySQL 5.7. We get the same issue as I noted in #235. I see more of what the problem is now. Because the order records are where the Acquisition Type is stored and we also show Acquisition Type on the index page in the table, we are going to have the problem of what to put in that field for the Resource. This might need to discussed among Steering Committee members if we need to remove the Acquisition Type field from the index table or if we need to allow multiple Resource records if the Acquisition Type changes between orders. |
|
Ok, so here's what I would do to keep MySQL 5.7 happy and to keep consistency in user display.
Is that ok with you? |
|
I'll test that nothing is broken and give my OK on the functionality. If others see this, I would defer to those who use that screen the most, since it is kind of a UI change. |
This patch allows multiple ordering in Coral, see Issue #243 for why it is needed.
Test plan:
resources/install/protected/update_Orders.sql, it will create the new ResourceAcquisition table and change existing tables to be linked to ResourceAcquisition, create a new default order for every resource in the database, update resource data to be linked to this default order, and delete now unused fields from the Resource table.You now have a new 'Order' panel for every resource in your database:
You can now create a new order for your resource. Note that when you have more than one order, a dropdown list will appear, right after the resource title, allowing you to select which order you want to display informations for:
You can also clone an existing order, allowing you to easily create a new order without having to input all informations again.
Now all informations in the panels below Product will be order specific.
You can have order-specific and resource-inherited contacts:
Please note that even though code and database changes are quite important, current users that do not need this feature will see almost no change to the interface when using the single default order.