Bug 449260

Summary:	Discover shouldn't be able to remove Discover
Product:	[Applications] Discover	Reporter:	WS <ws.kde>
Component:	discover	Assignee:	Dan Leinir Turthra Jensen <leinir>
Status:	RESOLVED FIXED
Severity:	normal	CC:	aleixpol, nate
Priority:	NOR	Keywords:	usability
Version:	unspecified
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:	https://invent.kde.org/plasma/discover/commit/bb064e8767970a0e8d32a2c531388f0d925b3d6c	Version Fixed In:	5.24

Description WS 2022-01-27 18:39:23 UTC

While the user should be able to uninstall Discover using other tools, Discover being able to nuke itself is putting too much faith on the user to not also uninstall other important things. 

To give an example: On my system(openSUSE Tumbleweed) it looks like I'm able to use Discover to uninstall Konsole, the GUI Package Manager(YaST) and then Discover itself. While this wouldn't make the system irreparable, it appears to be quite of an oversight.

It allows the fabled "Average User" to put himself into a situation he can't get out. It's just a disaster waiting to happen, and I can't help but think that a user that knows what he is doing wouldn't use Discover for the job.

Also side note: putting it inside the installed page is just kinda confusing, if Discover needs an "App page", it should be put as a link inside "about".

Comment 1 Nate Graham 2022-01-27 21:21:44 UTC

Yeah, this is probably not the kind of thing that a user of Discover should ever have a reason to do.

We should probably do two things here:

1. mark Discover as either "compulsory for desktop=kde" in its AppStream Metadata, or else manually add it to the critical packages list (I prefer the former)
2. Hide any critical packages from the UI, so you can't even see them at all.

Comment 2 Nate Graham 2022-01-27 21:26:07 UTC

Or maybe for #2, we should simply disable the "Remove" button instead of hiding it entirely. That would be a bit more HIG-compliant.

Comment 3 Bug Janitor Service 2022-01-27 21:33:02 UTC

A possibly relevant merge request was started @ https://invent.kde.org/plasma/discover/-/merge_requests/240

Comment 4 WS 2022-01-27 21:57:58 UTC

I'd rather go for the second option for really essential stuff that would make Discover or the system unusable without the User knowing what they're doing, because if they are tinkering at that level, they shouldn't use Discover.

Also, I'm not sure how Discover deals with critical packages, last I checked it was a warning, which I'm not a fan of, I think the "Remove" button shouldn't exist for critical packages, if they need to be shown.

Comment 5 Nate Graham 2022-01-27 22:01:21 UTC

Yep, the merge request I'm about to submit will do just that.

Comment 6 Bug Janitor Service 2022-01-27 22:02:37 UTC

A possibly relevant merge request was started @ https://invent.kde.org/plasma/discover/-/merge_requests/241

Comment 7 WS 2022-01-27 22:06:11 UTC

Yeah, this is way better. The old way meant that if a user knew packageX was critical, they would see a "Remove" Button on Discover, wouldn't click on it, and wouldn't know Discover had a way to prevent critical packages from being uninstalled.

They would just think Discover is dumb and unsafe.

Comment 8 Nate Graham 2022-01-27 22:10:46 UTC

Great!

Comment 9 Nate Graham 2022-01-29 21:03:56 UTC

Git commit d67837557bf68b2e801ac303184c7dfcefb4419c by Nate Graham.
Committed on 29/01/2022 at 21:03.
Pushed by ngraham into branch 'master'.

Mark Discover compulsory_for_desktop with KDE

That way anything within Discover that tries to uninstall Discover will
fail, since Discover checks for this sort of thing before allowing
uninstallation to proceed.
FIXED-IN: 5.24

M  +1    -0    discover/org.kde.discover.appdata.xml

https://invent.kde.org/plasma/discover/commit/d67837557bf68b2e801ac303184c7dfcefb4419c

Comment 10 Nate Graham 2022-01-29 21:04:22 UTC

Git commit bb064e8767970a0e8d32a2c531388f0d925b3d6c by Nate Graham.
Committed on 29/01/2022 at 21:04.
Pushed by ngraham into branch 'Plasma/5.24'.

Mark Discover compulsory_for_desktop with KDE

That way anything within Discover that tries to uninstall Discover will
fail, since Discover checks for this sort of thing before allowing
uninstallation to proceed.
FIXED-IN: 5.24


(cherry picked from commit d67837557bf68b2e801ac303184c7dfcefb4419c)

M  +1    -0    discover/org.kde.discover.appdata.xml

https://invent.kde.org/plasma/discover/commit/bb064e8767970a0e8d32a2c531388f0d925b3d6c

Comment 11 Nate Graham 2022-02-10 18:06:23 UTC

Git commit 456b8c1eb4e5f14bed6fc2780e999b4fe04da681 by Nate Graham.
Committed on 10/02/2022 at 18:04.
Pushed by ngraham into branch 'master'.

Don't display critical packages/apps in the UI

Right now when the user clicks the Remove button for these, they'll get
a big ugly error message. Instead, we should just not show them in the
UI at all.

M  +3    -2    libdiscover/backends/PackageKitBackend/PackageKitBackend.cpp

https://invent.kde.org/plasma/discover/commit/456b8c1eb4e5f14bed6fc2780e999b4fe04da681

Comment 12 Nate Graham 2022-02-10 18:08:05 UTC

Git commit 4e6748b1fc21bbd1f983adb78dcac95b7174706f by Nate Graham.
Committed on 10/02/2022 at 18:08.
Pushed by ngraham into branch 'Plasma/5.24'.

Don't display critical packages/apps in the UI

Right now when the user clicks the Remove button for these, they'll get
a big ugly error message. Instead, we should just not show them in the
UI at all.


(cherry picked from commit 456b8c1eb4e5f14bed6fc2780e999b4fe04da681)

M  +3    -2    libdiscover/backends/PackageKitBackend/PackageKitBackend.cpp

https://invent.kde.org/plasma/discover/commit/4e6748b1fc21bbd1f983adb78dcac95b7174706f