Skip to content

Commit c2f2d40

Browse files
authored
Merge pull request #2750 from nextcloud/fix/submit-error-toast
fix: show toast on errors with empty response
2 parents 24d1f52 + 51e3427 commit c2f2d40

3 files changed

Lines changed: 69 additions & 21 deletions

File tree

lib/Service/FormsService.php

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
use OCP\Search\ISearchQuery;
3535
use OCP\Security\ISecureRandom;
3636
use OCP\Share\IShare;
37+
use Psr\Log\LoggerInterface;
3738

3839
/**
3940
* Trait for getting forms information in a service
@@ -62,6 +63,7 @@ public function __construct(
6263
private IRootFolder $rootFolder,
6364
private IL10N $l10n,
6465
private IEventDispatcher $eventDispatcher,
66+
private LoggerInterface $logger,
6567
) {
6668
$this->currentUser = $userSession->getUser();
6769
}
@@ -562,19 +564,30 @@ public function getShareDisplayName(array $share): string {
562564
* @param Share $share The new Share
563565
*/
564566
public function notifyNewShares(Form $form, Share $share): void {
565-
switch ($share->getShareType()) {
566-
case IShare::TYPE_USER:
567-
$this->activityManager->publishNewShare($form, $share->getShareWith());
568-
break;
569-
case IShare::TYPE_GROUP:
570-
$this->activityManager->publishNewGroupShare($form, $share->getShareWith());
571-
break;
572-
case IShare::TYPE_CIRCLE:
573-
$this->activityManager->publishNewCircleShare($form, $share->getShareWith());
574-
break;
575-
default:
576-
// Do nothing.
567+
try {
568+
switch ($share->getShareType()) {
569+
case IShare::TYPE_USER:
570+
$this->activityManager->publishNewShare($form, $share->getShareWith());
571+
break;
572+
case IShare::TYPE_GROUP:
573+
$this->activityManager->publishNewGroupShare($form, $share->getShareWith());
574+
break;
575+
case IShare::TYPE_CIRCLE:
576+
$this->activityManager->publishNewCircleShare($form, $share->getShareWith());
577+
break;
578+
default:
579+
// Do nothing.
580+
}
581+
} catch (\Exception $e) {
582+
// Handle exceptions silently, as this is not critical.
583+
// We don't want to break the share creation process just because of an activity error.
584+
$this->logger->error(
585+
'Error while publishing new share activity',
586+
[$e]
587+
);
588+
577589
}
590+
578591
}
579592

580593
/**
@@ -585,14 +598,31 @@ public function notifyNewShares(Form $form, Share $share): void {
585598
*/
586599
public function notifyNewSubmission(Form $form, Submission $submission): void {
587600
$shares = $this->getShares($form->getId());
588-
$this->activityManager->publishNewSubmission($form, $submission->getUserId());
601+
try {
602+
$this->activityManager->publishNewSubmission($form, $submission->getUserId());
603+
} catch (\Exception $e) {
604+
// Handle exceptions silently, as this is not critical.
605+
// We don't want to break the submission process just because of an activity error.
606+
$this->logger->error(
607+
'Error while publishing new submission activity',
608+
[$e]
609+
);
610+
}
589611

590612
foreach ($shares as $share) {
591613
if (!in_array(Constants::PERMISSION_RESULTS, $share['permissions'])) {
592614
continue;
593615
}
594-
595-
$this->activityManager->publishNewSharedSubmission($form, $share['shareType'], $share['shareWith'], $submission->getUserId());
616+
try {
617+
$this->activityManager->publishNewSharedSubmission($form, $share['shareType'], $share['shareWith'], $submission->getUserId());
618+
} catch (\Exception $e) {
619+
// Handle exceptions silently, as this is not critical.
620+
// We don't want to break the submission process just because of an activity error.
621+
$this->logger->error(
622+
'Error while publishing new shared submission activity',
623+
[$e]
624+
);
625+
}
596626
}
597627

598628
$this->eventDispatcher->dispatchTyped(new FormSubmittedEvent($form, $submission));

src/views/Submit.vue

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -818,12 +818,21 @@ export default {
818818
this.deleteFormFieldFromLocalStorage()
819819
emit('forms:last-updated:set', this.form.id)
820820
} catch (error) {
821+
const errorMessage = error.response?.data?.ocs?.meta?.message
821822
logger.error('Error while submitting the form', { error })
822-
showError(
823-
t('forms', 'There was an error submitting the form: {message}', {
824-
message: error.response.data.ocs.meta.message,
825-
}),
826-
)
823+
if (errorMessage) {
824+
showError(
825+
t(
826+
'forms',
827+
'There was an error submitting the form: {message}',
828+
{
829+
message: errorMessage,
830+
},
831+
),
832+
)
833+
} else {
834+
showError(t('forms', 'There was an error submitting the form'))
835+
}
827836
} finally {
828837
this.loading = false
829838
if (!this.publicView) {

tests/Unit/Service/FormsServiceTest.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,9 @@ function microtime(bool|float $asFloat = false) {
5858
use OCP\IUserSession;
5959
use OCP\Security\ISecureRandom;
6060
use OCP\Share\IShare;
61-
6261
use PHPUnit\Framework\MockObject\MockObject;
62+
63+
use Psr\Log\LoggerInterface;
6364
use Test\TestCase;
6465

6566
class FormsServiceTest extends TestCase {
@@ -105,6 +106,8 @@ class FormsServiceTest extends TestCase {
105106

106107
/** @var IL10N|MockObject */
107108
private $l10n;
109+
/** @var LoggerInterface|MockObject */
110+
private $logger;
108111

109112
public function setUp(): void {
110113
parent::setUp();
@@ -120,6 +123,7 @@ public function setUp(): void {
120123
$this->userManager = $this->createMock(IUserManager::class);
121124
$this->secureRandom = $this->createMock(ISecureRandom::class);
122125
$this->circlesService = $this->createMock(CirclesService::class);
126+
$this->logger = $this->createMock(LoggerInterface::class);
123127
$userSession = $this->createMock(IUserSession::class);
124128

125129
$user = $this->createMock(IUser::class);
@@ -155,6 +159,7 @@ public function setUp(): void {
155159
$this->storage,
156160
$this->l10n,
157161
\OCP\Server::get(IEventDispatcher::class),
162+
$this->logger,
158163
);
159164
}
160165

@@ -632,6 +637,7 @@ public function testGetPermissions_NotLoggedIn() {
632637
$this->storage,
633638
$this->l10n,
634639
\OCP\Server::get(IEventDispatcher::class),
640+
$this->logger,
635641
);
636642

637643
$form = new Form();
@@ -871,6 +877,7 @@ public function testPublicCanSubmit() {
871877
$this->storage,
872878
$this->l10n,
873879
\OCP\Server::get(IEventDispatcher::class),
880+
$this->logger,
874881
);
875882

876883
$this->assertEquals(true, $formsService->canSubmit($form));
@@ -982,6 +989,7 @@ public function testHasUserAccess_NotLoggedIn() {
982989
$this->storage,
983990
$this->l10n,
984991
\OCP\Server::get(IEventDispatcher::class),
992+
$this->logger,
985993
);
986994

987995
$form = new Form();
@@ -1213,6 +1221,7 @@ public function testNotifyNewSubmission($shares, $shareNotifications) {
12131221
$this->storage,
12141222
$this->l10n,
12151223
$eventDispatcher,
1224+
$this->logger,
12161225
])
12171226
->getMock();
12181227

0 commit comments

Comments
 (0)