Opened 10 years ago

Closed 10 years ago

#2087 closed enhancement (fixed)

[Enhancement] Protected Activities

Reported by: tch Owned by: tch
Priority: Unspecified by Maintainer Milestone: Unspecified
Component: Sugar Version: Unspecified
Severity: Unspecified Keywords: sugar-love r+ dextrose
Cc: rgs_, jasg, bernie, tomeu Distribution/OS: Unspecified
Bug Status: Unconfirmed

Description

Teachers complain about their kids deleting educational activities. Their argument is that they lose to much time because of this. Therefore sugar should allow deployments to define a protected activities list. Protected activities should not be able to get deleted from the GUI.

Change History (24)

Changed 10 years ago by tch

comment:1 Changed 10 years ago by tch

  • Keywords r? added

comment:2 Changed 10 years ago by tch

  • Keywords sugar-love added; sugr-love removed

comment:3 Changed 10 years ago by garycmartin

I believe this feature is already implemented with current Sugar, just testing in 0.88 here (but think this goes back a little while, not sure about 0.84 given all the back-porting that's been going on).

I have just tested with Browse.activity protected, you can remove as a favourite with the home list view, but you can not erase it (erase menu item does not show). Trick seems to be to put the activity in /usr/share/sugar/activities, Sugar then removes the erase option. From what I remember the activity can still be upgraded (~/Activities overrides a version in /usr/share/sugar/activities), but I've not tested much in that area. The SoaS distro has been using this feature for the last couple of releases.

The other option is to change the ownership permission of the ~/Activities activity, Sugar will then disable the Erase menu item (it's still there but is greyed out). This approach prevents the user from getting an activity upgraded, so I'd not recommend it if you value some level of freedom (but a nice touch that the UI indicates it can't erase something).

comment:4 Changed 10 years ago by tch

Yes, Sugar already has a differentiation between user's activities and system activities, anything outside ~/ (apparently) it is consider an system activity. The problem with that solution is that you get duplicated activities (in the fs) while upgrading, wasting storage space.

Changing the owner of the activity files also has problems because it does not let you upgrade the activity version.

Even if there still some corner cases that i will solve, this solution does not waste storage space and it lets the user to upgrade the activity. Because it is a very simple and superficial solution that is all we should do, because users should have root access anyways, so there is no REAL way to avoid this happening, at the end of the day. :)

comment:5 follow-up: Changed 10 years ago by tomeu

  • Keywords r! added; r? removed

Three questions:

  • have you considered using GConf so deployments can change the default without patching rpms? May be good to avoid one more configuration file.
  • do we want to hide the menu option or to grey it out like we do when we cannot write to the dir?
  • don't we want two separate error messages when we fail to read either the favorite defaults or the protected activities?

s/protects_activity/is_activity_protected

comment:6 in reply to: ↑ 5 ; follow-up: Changed 10 years ago by tch

Replying to tomeu:

Three questions:

  • have you considered using GConf so deployments can change the default without patching rpms? May be good to avoid one more configuration file.

I see, I did use gconf in the first patch version, just though it would be confusing to have some activities list configurations in text files and others in gconf. Plus, I think I have seen some discussions about the future of gconf. That's why I used a text file.

  • do we want to hide the menu option or to grey it out like we do when we cannot write to the dir?

Honestly, I don't think it makes any difference. If theres some good argument I don't know, please let me know and I'll change it.

  • don't we want two separate error messages when we fail to read either the favorite defaults or the protected activities?

s/protects_activity/is_activity_protected

Just did not like the idea of having 2 try blocks (one after another) doing "almost" the same thing, but surely I can change that if you don't agree. :)

comment:7 Changed 10 years ago by tch

  • Keywords r? dextrose added; r! removed

Please also review:

Delete profile data when removing activities: http://bugs.sugarlabs.org/ticket/2074
Deleting .xo entries from journal corner case: http://bugs.sugarlabs.org/ticket/1512

comment:8 in reply to: ↑ 6 Changed 10 years ago by tomeu

  • Keywords r! added; r? removed

Replying to tch:

Replying to tomeu:

Three questions:

  • have you considered using GConf so deployments can change the default without patching rpms? May be good to avoid one more configuration file.

I see, I did use gconf in the first patch version, just though it would be confusing to have some activities list configurations in text files and others in gconf. Plus, I think I have seen some discussions about the future of gconf. That's why I used a text file.

Ok, but having to spin a patched rpm is some orders of magnitude less convenient than dropping a file in /etc/gconf. Also, GConf is being replaced with dconf which is quite similar in this respect, we'll migrate all the settings from gconf when the time comes.

  • do we want to hide the menu option or to grey it out like we do when we cannot write to the dir?

Honestly, I don't think it makes any difference. If theres some good argument I don't know, please let me know and I'll change it.

This should have been asked to the design team before you started to code.

  • don't we want two separate error messages when we fail to read either the favorite defaults or the protected activities?

s/protects_activity/is_activity_protected

Just did not like the idea of having 2 try blocks (one after another) doing "almost" the same thing, but surely I can change that if you don't agree. :)

Good error reporting trumps code succintness almost always.

comment:9 Changed 10 years ago by tch

  • Keywords r? added; r! removed

switched to gconf now :)

comment:10 Changed 10 years ago by bernie

Ping?

comment:11 Changed 10 years ago by seeta

  • Owner changed from tch to kandarpk
  • Status changed from new to assigned

comment:12 follow-up: Changed 10 years ago by dsd

When I saw this idea originally I didn't like it, but I've now warmed up to it.

Deletion of activities is certainly a big problem in OLPC deployments to the point where at least 2 of them (Nicaragua, Peru) have completely removed the "Erase activity" option, for all activities (clearly a sub-standard "solution" to the problem).

The ideal solution needs to:

  1. Allow the deployment to prevent deletion of a specific set of activities - the ones that are covered in teacher training, the ones that lesson plans are provided for, etc.
  2. Not hinder the user's ability to download additional activities, and delete them at their will

This patch meets both requirements and the implementation has my vote (I've reviewed the patch).

As far as I can see, the only pending task is to get a comment from the design team about hiding or greying out the option. As they are responsive and it isn't a big decision, this should only require a couple of quick emails.

comment:13 in reply to: ↑ 12 Changed 10 years ago by garycmartin

Replying to dsd:

When I saw this idea originally I didn't like it, but I've now warmed up to it.

Yes, same here.

The ideal solution needs to:

  1. Allow the deployment to prevent deletion of a specific set of activities - the ones that are covered in teacher training, the ones that lesson plans are provided for, etc.
  2. Not hinder the user's ability to download additional activities, and delete them at their will

... my fav is the no 3. Allow user to upgrade a locked activity to a new release when it is made available.

As far as I can see, the only pending task is to get a comment from the design team about hiding or greying out the option. As they are responsive and it isn't a big decision, this should only require a couple of quick emails.

FWIW I don't see a strong case, actually to be honest I don't really like the hidden vs. greyed out design we have now, seems a fairly meangless and arbetairy distinction that is perhaps only sane in a thin client setup. Just greying out in all cases would seem to be the simple choice and keep the menus consistent. Anyway, given the fairly weak case, if the activity is installed in ~\Activities, the erase item should be shown but greyed out, as technically it is in the users home and they could just go edit the protected list and then erase the file anyway.

comment:14 Changed 10 years ago by dsd

The patch also meets requirement #3

So...next step would be a quick respin of the patch that greys-out the option rather than hiding it.

comment:15 Changed 10 years ago by tch

  • Cc rgs_ jasg bernie tomeu added; rgs_ jasg bernie removed

Thanks to alsroot, silbe and Kandarp Kaushik for the comment and improvements,

comment:16 Changed 10 years ago by dsd

Looks great, has my vote.

Tomeu, I think this patch is ready for your final judgement.

comment:17 Changed 10 years ago by tomeu

  • Keywords r+ added; r? removed

Only one change needed:

+        self._protected_activities = client.get_list('/desktop/sugar/protected_activities',
+                                                    gconf.VALUE_STRING)

Line too long, needs to be split.

+        if not activity_info.is_user_activity():
+            return

Would prefer this outside the function.

comment:18 Changed 10 years ago by tch

  • Owner changed from kandarpk to tch

comment:19 Changed 10 years ago by tch

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.