Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multiple issues with resource submission #390

Merged
merged 2 commits into from
Apr 6, 2018

Conversation

veggiematts
Copy link
Contributor

@veggiematts veggiematts commented Mar 22, 2018

  • Don't create multiple orders when a resource is saved as draft before being submitted
    Test plan:

    • Before the fix: Create a resource. Save it as draft, validate the resource. Two default orders will be created instead of one. If you edit the resource as draft multiple times before validating it, multiple default orders will be created.

    • After the fix: Proceed as before. Check that only one default order has been created when the resource is validated.

  • Fix acquisitionType selection when editing a resource saved as draft
    Test plan:

    • Before the fix: Create a resource with an acquisitionType. Save it as draft. Edit it as draft. The acquisitionType will not be selected in the form.
    • After the fix: Proceed as before. Check that the acquisitionType is selected when editing the resource as draft. Check that the acquisitionType is registered in the default order when the resource is validated.
  • Remove dead code: a ResourcePayment array was created and never used.

 - Don't create multiple orders when a resource is saved as draft before being submitted
 - Fix acquisitionType selection when editing a resource saved as draft
 - Remove dead code
@veggiematts veggiematts added the bug This is a bug (not an enhancement) label Mar 22, 2018
@veggiematts veggiematts added this to the Version 3.0.0 Beta milestone Mar 22, 2018
@veggiematts veggiematts added the important Not critical but should be addressed label Mar 28, 2018
@jeffnm
Copy link
Contributor

jeffnm commented Apr 6, 2018

This is the last open PR tagged for 3.0 with an important label. I'll try to take a look over the weekend and see if I can test it, unless someone else beats me to it, or it gets moved to the parking lot for this beta.

@t4k
Copy link
Contributor

t4k commented Apr 6, 2018

I was getting a PHP Fatal Error when testing this:

Fatal error: Uncaught Exception: There was a problem with the database: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'ORDER BY noteTypeID desc LIMIT 0,1' at line 4 in /var/www/coral/web/resources/admin/classes/common/DBService.php:52 Stack trace: #0 /var/www/coral/web/resources/admin/classes/common/DBService.php(91): DBService->checkForError() #1 /var/www/coral/web/resources/admin/classes/domain/Resource.php(373): DBService->processQuery('SELECT * FROM R...', 'assoc') #2 /var/www/coral/web/resources/admin/classes/common/Object.php(55): Resource->getInitialNote() #3 /var/www/coral/web/resources/ajax_forms/getNewResourceForm.php(54): Object->__get('getInitialNote') #4 /var/www/coral/web/resources/ajax_forms.php(24): include('/var/www/coral/...') #5 {main} thrown in /var/www/coral/web/resources/admin/classes/common/DBService.php on line 52

I committed a fix that adds quotes around the nodeTypeID in the SQL.

Otherwise this PR fixes the issues as laid out in the test plan.

@t4k
Copy link
Contributor

t4k commented Apr 6, 2018

I'm probably the last one working today, so I'll go ahead and merge this, but anyone else can test it out after it is merged as well.

@t4k t4k merged commit 5491768 into coral-erm:development Apr 6, 2018
@scottvieira
Copy link
Contributor

@t4k Was this a bug introduced with the multiple orders feature?

@t4k
Copy link
Contributor

t4k commented Apr 10, 2018

@scottvieira Yes, it looks like it was introduced in #244.

@veggiematts
Copy link
Contributor Author

Yes, it was.

@scottvieira
Copy link
Contributor

@t4k and @veggiematts Thank you both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a bug (not an enhancement) important Not critical but should be addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants