Sharer gets "Email notification was sent!" message even when the email was not sent

[paurakhsharma]

Steps to reproduce

Scenario 1: user should get an error message when trying to send notification by email to a user who has not setup their email
    Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "yes"
    And user "user1" has logged in using the webUI
    And user "user1" has shared file "lorem.txt" with user "user2"
    And the user has opened the share dialog for file "lorem.txt"
    When the user sends the share notification by email using the webUI
    Then a notification should be displayed on the webUI with the text "Email notification was not sent!"

• Note: In the first scenario the email of the receiver is not set.

Scenario 2: user should get an error message when trying to send notification when email server setting is invalid
    Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "yes"
    And user "user1" has logged in using the webUI
    And user "user1" has shared file "lorem.txt" with user "user2"
    And the user has opened the share dialog for file "lorem.txt"
    When the user sends the share notification by email using the webUI
    Then a notification should be displayed on the webUI with the text "Email notification was not sent!"

• Note: In the second scenario set the receiver's email but change your Email Server setting in Admin General settings page to invalid one.

Actual behaviour

User should get an error message saying "Email notification was sent!"

Expected behaviour

User should get an error message saying "Email notification was not sent!"

c.c @individual-it @phil-davis @PVince81

Response in DevTools

{"ocs":{"meta":{"status":"ok","statuscode":200,"message":"Couldn't send mail to following recipient(s): arkouser ","totalitems":"","itemsperpage":""},"data":[]}}

[paurakhsharma]

It turns out after you click on "notify by email" you get a message "Email notification was sent!"
which you should not and if you refresh the page you get to click on the "notify by email" again
which you should not if the email was actually sent.

[paurakhsharma]

Could not reproduce in ''10.1"

So this is regression

[paurakhsharma]

This is the network response in "10.1"
{"data":{"message":"Couldn't send mail to following recipient(s): arkouser "},"status":"error"}

Here the status is "error" compared to "ok" in 10.2 RC2 and RC3

[PVince81]

@VicDeo

[VicDeo]

ok, it was a mistake to rely on result.ocs.meta.status when parsing a result.
ocs.meta.status is 200 even when some emails were not sent:

{"ocs":{"meta":{"status":"ok","statuscode":200,"message":"Couldn't send mail to following recipient(s): arkouser ","totalitems":"","itemsperpage":""},"data":[]}}

[PVince81]

@VicDeo why not fix the backend to return the correct value ?

how much of the refactoring/cleanup work that we backported touches this ? did it touch the frontend or only backend ?

[VicDeo]

@PVince81 a470ac4#diff-e8d8565fb4fcb24d2ff551b5e127cf6a

[PVince81]

thanks for the justification, makes sense

[VicDeo]

@PVince81 I can mimic the prev behavior but I need help with unsuccessful OCS status clarification.
An old endpoint sent {'status':'success'} or {'status':'error'} via deprecated OCP\JSON class
May be I need just to include the similar field into the ocs response?

[PVince81]

@VicDeo adding the field in the OCS response makes sense.

if that doesn't work out for whatever reason, add a "FIXME" then in the PR