search

pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
297 stars 442 forks source link

Removing template from mailable returns 500 #10094

Open jardakotesovec opened 2 weeks ago

jardakotesovec commented 2 weeks ago

Describe the bug Removing template from mailable returns 500

Affect only main branch

To Reproduce Steps to reproduce the behavior:

  1. Go to Go to Wofkflow -> Emails -> Add and edit templates
  2. Click on mailable which has templates, for example Discussion (Copyediting)
  3. Click on Remove on any of the templates and confirm dialog to delete template
  4. See error in network console - endpoint returning 500, with following stacktrace:
[Wed Jun 19 12:01:18 2024] [::1]:60856 Accepted
[Wed Jun 19 12:01:18 2024] Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException: The DELETE method is not supported for route publicknowledge/api/v1/emailTemplates/LAYOUT_COMPLETE. Supported methods: GET, HEAD, PUT. in /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Routing/AbstractRouteCollection.php:122
Stack trace:
#0 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Routing/AbstractRouteCollection.php(107): Illuminate\Routing\AbstractRouteCollection->requestMethodNotAllowed(Object(Illuminate\Http\Request), Array, 'DELETE')
#1 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Routing/AbstractRouteCollection.php(41): Illuminate\Routing\AbstractRouteCollection->getRouteForMethods(Object(Illuminate\Http\Request), Array)
#2 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Routing/RouteCollection.php(162): Illuminate\Routing\AbstractRouteCollection->handleMatchedRoute(Object(Illuminate\Http\Request), NULL)
#3 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/classes/core/PKPBaseController.php(106): Illuminate\Routing\RouteCollection->match(Object(Illuminate\Http\Request))
#4 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/classes/core/PKPBaseController.php(114): PKP\core\PKPBaseController::getRequestedRoute(Object(Illuminate\Http\Request))
#5 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/classes/middleware/PolicyAuthorizer.php(43): PKP\core\PKPBaseController::getRouteController(Object(Illuminate\Http\Request))
#6 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): PKP\middleware\PolicyAuthorizer->handle(Object(Illuminate\Http\Request), Object(Closure))
#7 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#8 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ConvertEmptyStringsToNull.php(31): Illuminate\Foundation\Http\Middleware\TransformsRequest->handle(Object(Illuminate\Http\Request), Object(Closure))
#9 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull->handle(Object(Illuminate\Http\Request), Object(Closure))
#10 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#11 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TrimStrings.php(40): Illuminate\Foundation\Http\Middleware\TransformsRequest->handle(Object(Illuminate\Http\Request), Object(Closure))
#12 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\Foundation\Http\Middleware\TrimStrings->handle(Object(Illuminate\Http\Request), Object(Closure))
#13 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php(27): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#14 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\Foundation\Http\Middleware\ValidatePostSize->handle(Object(Illuminate\Http\Request), Object(Closure))
#15 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/classes/middleware/ValidateCsrfToken.php(46): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#16 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): PKP\middleware\ValidateCsrfToken->handle(Object(Illuminate\Http\Request), Object(Closure))
#17 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/classes/middleware/DecodeApiTokenWithValidation.php(58): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#18 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): PKP\middleware\DecodeApiTokenWithValidation->handle(Object(Illuminate\Http\Request), Object(Closure))
#19 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/classes/middleware/SetupContextBasedOnRequestUrl.php(65): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#20 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): PKP\middleware\SetupContextBasedOnRequestUrl->handle(Object(Illuminate\Http\Request), Object(Closure))
#21 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/classes/middleware/AllowCrossOrigin.php(34): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#22 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): PKP\middleware\AllowCrossOrigin->handle(Object(Illuminate\Http\Request), Object(Closure))
#23 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(116): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#24 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/classes/handler/APIHandler.php(63): Illuminate\Pipeline\Pipeline->then(Object(Closure))
#25 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/api/v1/emailTemplates/index.php(18): PKP\handler\APIHandler->__construct(Object(PKP\API\v1\emailTemplates\PKPEmailTemplateController))
#26 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/classes/core/APIRouter.php(93): require('/Users/jarda/gi...')
#27 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/classes/core/Dispatcher.php(161): PKP\core\APIRouter->route(Object(APP\core\Request))
#28 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/lib/pkp/classes/core/PKPApplication.php(379): PKP\core\Dispatcher->dispatch(Object(APP\core\Request))
#29 /Users/jarda/git/pkp/pkp-main-2/ojs-main-2/index.php(21): PKP\core\PKPApplication->execute()
#30 {main}
[Wed Jun 19 12:01:18 2024] [::1]:60856 [500]: POST /index.php/publicknowledge/api/v1/emailTemplates/LAYOUT_COMPLETE

What application are you using? OJS, OMP or OPS version main

Additional information

Screenshot 2024-06-19 at 12 01 27
kaitlinnewson commented 2 weeks ago

I'm able to reproduce this issue, but one thing I'd like to clarify is if the user should be able to delete these templates in the UI. In our docs, we say:

Default templates are described in emailTemplates.xml. These templates are installed when the application is installed or a new locale is added.

These templates can not be edited or deleted. [...]

Many of the templates I'm testing with are in emailTemplates.xml. Should the "Remove" button appear for these?

Screenshot 2024-06-21 at 2 33 12 PM

kaitlinnewson commented 2 weeks ago

Ready for review @asmecher:

jardakotesovec commented 2 weeks ago

Would be also interested to know the intention whether there should be different behaviour between templates that are coming from emailTemplates.xml and the ones that are added by the user.

I noticed there also should be option to reset template (manageEmails.tpl) - for some templates:

                    <pkp-button
                        v-if="item.key === currentMailable.emailTemplateKey && item.id"
                        :is-warnable="true"
                        @click="confirmResetTemplate(item)"
                    >
                        {translate key="common.reset"}
                    </pkp-button>

My first guess would be that the ones coming from emailTemplates.xml should be possible to edit and reset, but not remove. And the ones from user would be possible to edit and remove.

Maybe @NateWr remembers? :-)

jardakotesovec commented 2 weeks ago

@Vitaliy-1 Or maybe you would know?

asmecher commented 2 weeks ago

(Meanwhile, I did merge the PR above. I agree that it's overspecific for the email template keys we use.)

Vitaliy-1 commented 1 week ago

My first guess would be that the ones coming from emailTemplates.xml should be possible to edit and reset, but not remove.

Yes, those are default templates, it's impossible to modify them directly. They are stored in the email_templates_default_data table. When user changes template through the UI or adds a new, it's recorded into email_templates