biigle / reports

:m: BIIGLE module to generate reports for projects and volumes
GNU General Public License v3.0
1 stars 2 forks source link

Resolution for Issue #73 -- Returning Report ID through API. #78

Closed rskelly closed 2 years ago

rskelly commented 2 years ago

Updates to allow suppression of email notification when reports are requested from the API by adding a persistable notify_when_ready attribute to the Report object.

Possible problem: the isAutomatedRequest method checks the ajax, wantsJson and getUser return values to determine whether the request is automatic, but in my testing, json and ajax are true and user is null when a request is made from the UI; and json and ajax are false and user is not null when a request is made from the API. So the user object is checked to make the determination, as isAutomatedRequest always returns true.

Pull request includes a migration and test for both possible values of notify_when_ready.

Resolves #73

rskelly commented 2 years ago

@mzur Not sure if this is the right place to ask this, but do you have any suggestions for testing this line?

$disableNotifications = $this->report->options['disableNotifications'] ?? false;

I get this error, but can't seem to find any information about either configuring an expectation for accessing the options array, or bypassing it.

BadMethodCallException: Received Mockery_2_Biigle_Modules_Reports_Report::offsetExists(), but no expectations were specified
mzur commented 2 years ago

What about this?

$report->shouldReceive('getAttribute')->with('options')->andReturn(['disableNotifications' => true]);

If this doesn't work and you are not sure what to do then I can finish the tests, too.

rskelly commented 2 years ago

Thanks for the feedback. That didn't do it, but an expectation for offsetExists and then get/set attribute seems to do the trick. Added tests for true/false/none.

Thanks for your patience :)