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

[4.0] Core API - Delete #31130

Closed
Razzo1987 opened this issue Oct 17, 2020 · 16 comments
Closed

[4.0] Core API - Delete #31130

Razzo1987 opened this issue Oct 17, 2020 · 16 comments

Comments

@Razzo1987
Copy link
Contributor

Steps to reproduce the issue

Use Joomla 4 COre API - Banner (this example can help you: https://documenter.getpostman.com/view/9617873/Szzj9yJy#f8b4dece-cb75-44a5-bf6b-b5d5f9b9d75b)

  1. With POST create a banner
  2. Check the ID with GET request
  3. Try to delete the banner with DELETE

Expected result

No response and the Banner is deleted

Actual result

Error 500
And if you enable the debugger a response like:

{
    "errors": [
        {
            "code": 500,
            "title": "Internal server error",
            "detail": "RuntimeException: There was an error deleting the item. in /var/www/vhosts/jugmi.it/subdomains/j4/httpdocs/libraries/src/MVC/Controller/ApiController.php:319\nStack trace:\n#0 /var/www/vhosts/jugmi.it/subdomains/j4/httpdocs/libraries/src/MVC/Controller/BaseController.php(729): Joomla\\CMS\\MVC\\Controller\\ApiController->delete()\n#1 /var/www/vhosts/jugmi.it/subdomains/j4/httpdocs/libraries/src/Dispatcher/ApiDispatcher.php(59): Joomla\\CMS\\MVC\\Controller\\BaseController->execute('delete')\n#2 /var/www/vhosts/jugmi.it/subdomains/j4/httpdocs/libraries/src/Component/ComponentHelper.php(389): Joomla\\CMS\\Dispatcher\\ApiDispatcher->dispatch()\n#3 /var/www/vhosts/jugmi.it/subdomains/j4/httpdocs/libraries/src/Application/ApiApplication.php(344): Joomla\\CMS\\Component\\ComponentHelper::renderComponent('com_banners')\n#4 /var/www/vhosts/jugmi.it/subdomains/j4/httpdocs/libraries/src/Application/ApiApplication.php(110): Joomla\\CMS\\Application\\ApiApplication->dispatch()\n#5 /var/www/vhosts/jugmi.it/subdomains/j4/httpdocs/libraries/src/Application/CMSApplication.php(233): Joomla\\CMS\\Application\\ApiApplication->doExecute()\n#6 /var/www/vhosts/jugmi.it/subdomains/j4/httpdocs/api/includes/app.php(54): Joomla\\CMS\\Application\\CMSApplication->execute()\n#7 /var/www/vhosts/jugmi.it/subdomains/j4/httpdocs/api/index.php(35): require_once('/var/www/vhosts...')\n#8 {main}"
        }
    ]
}

System information (as much as possible)

Joomla 4.0 nightly build Saturday, 17 October 2020 02:01:01 UTC

Additional comments

Same problem of many other DELETE API. Not on Article

@alikon
Copy link
Contributor

alikon commented Oct 17, 2020

it seems that for DELete a banner the banner item should be setted before to status -2 ie trashed then it works
the strange thing is that for articles you can delete it without transition to Trashed status before @wilsonge any opinion ?

@alikon
Copy link
Contributor

alikon commented Oct 17, 2020

@wilsonge
Copy link
Contributor

I originally deliberately allowed you to skip. But I'm not 100% - wondering - especially with workflows - I guess we can't have it insta deleting?

@alikon
Copy link
Contributor

alikon commented Oct 17, 2020

so content & workflow apart.... the other DELETE should be allowed to skip trash ??

@wilsonge
Copy link
Contributor

Im not sure. Generally that’s how api’s work. I’d be open to hear more people’s opinions though

@rdeutz
Copy link
Contributor

rdeutz commented Oct 27, 2020

I would say removing something fully that is published is not a good idea even over an api. So delete should move it into trash and when you delete again then it should be removed fully. Same as the application does it.

@Razzo1987
Copy link
Contributor Author

So, can we add a text to the error message like "You can not DELETE a not trashed item" ?

@alikon we need to change the DELETE of Articles (now directly delete). @rdeutz correct?

@alikon
Copy link
Contributor

alikon commented Oct 27, 2020 via email

@wilsonge
Copy link
Contributor

wilsonge commented Dec 3, 2020

OK let's remove the exceptions for the API from com_content...

@alikon
Copy link
Contributor

alikon commented Dec 4, 2020

if we do it in this way .... then you can only delete a trashed content item.... we don't have an entrypoint for "move to trash"...
....i dislike this choice.

@alikon
Copy link
Contributor

alikon commented Dec 4, 2020

see #31581

@HLeithner
Copy link
Member

API should follow the constrains a user has normally also plugin triggers that expect a trash before delete could have unexpected behavior

@alikon
Copy link
Contributor

alikon commented Dec 4, 2020

No, not in my view

@wilsonge
Copy link
Contributor

wilsonge commented Dec 4, 2020

we don't have an entrypoint for "move to trash"

We do - in the standard PATCH request to update an article you change the article state to trashed (or the appropriate workflow if you have workflows enabled)

API should follow the constrains a user has normally also plugin triggers that expect a trash before delete could have unexpected behavior

APIs definitely can shortcut some things in the UI. There's lots of precedents for that in public APIs. Having said that I dunno if it's in the best interests of the CMS to do that or whether it's better for people to follow the same process. Given workflows should enforce some of this anyhow - it's probably safer to do things the same way as the UI.

@alikon
Copy link
Contributor

alikon commented Dec 4, 2020

ok but in this way you need to call the Api 2 times ....

@alikon
Copy link
Contributor

alikon commented Dec 13, 2020

let's close this as #31581 it's merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants