208 - Edit course properties

[JHilker]

Quick preliminary PR for ticket https://github.com/FinalsClub/karmaworld/issues/208

Since we have no examples of doing a AJAX edit in the application, and I have no experience with django before this, I took a stab at what I thought might be best practices (after a few iterations). Looking for any feedback.

I've created an edit icon also, but the font is still a bit off, anyone know what font the other icons (like flag) use?

Ticket still needs: UPSERT - The definition of UPSERT that I know is that it is simply an update or insert call, and either updates an object if found, or creates it. It sounds like what we want however is versioning, where the latest version is what is used.

Course name slug - Should this slug be modified from the original if the course name changes? Con - current page url would be broken, people bookmarks might break. Pro - url properly represents course. I'm assuming we should, but figured I'd double check.

Course Name - School uniqueness validation. It appears we have no validator for this (it breaks on add too) We should probably have one in a general place, haven't looked into doing this yet.

First comment regarding note tag display + edit - Should probably be it's own ticket.

Requirement to be able to do this - Email regarding privileges states that editing the course meta data should require karma. I assume this means login as well. How much karma should be required to edit courses?

[btbonval]

Hrm, maybe UPSERT isn't the right word, because wikipedia basically says it is nothing but the usual create or update.

In the parlance I've used with other database devs, UPSERT treats UPDATEs as INSERTs. That is to say, if you update record A, then instead of updating A, you create a new record B that contains what A would have been modified to. Somehow B is marked as newer or more correct than A. DELETEs may be similarly handled with some kind of INSERT relevant to record A which marks a flag saying that A is deleted. Usually the tables contain logs of <timestamp, primary key> together as the composite primary key, and instead of looking for record A, you would look for the max(timestamp) on all A-like records in the log table.

This is done in any system where destructive UPDATEs/DELETEs must be avoided. It's a recovery mechanism of sorts. DELETE and UPDATE are unrecoverable. When you open the site to anonymous DELETE and anonymous UPDATE (or many other reasons), you really don't want adversaries to be able to destroy data that was previously there in an unrecoverable way. Backups are great, but even they aren't an adequate answer unless the capability exists to check 100% of the database prior to the periodic rollover that backups experience.

Think of it like Wikipedia itself. You don't simply UPDATE a Wikipedia page when you edit it. Although you're editing it, the database gets an INSERT. Look at the history log on Wikipedia and you'll see exactly what I mean when I say UPSERT. Timestamped entries that can be viewed historically or reverted to as needed when things get icky.

Anyway, you shouldn't need to implement functionality like that yourself. It should already exist somewhere. Unfortunately it looks like UPSERT might not be the right name, at least not according to wikipedia's brief high level summary. I know Rails this feature built in. I should find what they call it and do a search for that against Django. -Bryan

On Sat, Feb 1, 2014 at 4:52 PM, Jacob Hilker notifications@github.comwrote:

Quick preliminary PR for ticket #208https://github.com/FinalsClub/karmaworld/issues/208

Since we have no examples of doing a AJAX edit in the application, and I have no experience with django before this, I took a stab at what I thought might be best practices (after a few iterations). Looking for any feedback.

Ticket still needs: UPSERT - The definition of UPSERT that I know is that it is simply an update or insert call, and either updates an object if found, or creates it. It sounds like what we want however is versioning, where the latest version is what is used.

Course name slug - Should this slug be modified from the original if the course name changes? Con - current page url would be broken, people bookmarks might break. Pro - url properly represents course. I'm assuming we should, but figured I'd double check

Icon - I need to make an edit icon, should be simple enough.

Course Name - School uniqueness validation. It appears we have no validator for this (it breaks on add too) We should probably have one in a general place, haven't looked into doing this yet.

First comment regarding note tag display + edit - Should probably be it's own ticket.

Requirement to be able to do this - Email regarding privileges states that editing the course meta data should require karma. I assume this means

login as well. How much karma should be required to edit courses?

You can merge this Pull Request by running

git pull https://github.com/FinalsClub/karmaworld 208-edit-course-properties-main

Or view, comment on, or merge it at:

https://github.com/FinalsClub/karmaworld/pull/314 Commit Summary

• 208 - Edit course properties - first pass

File Changes

• M karmaworld/apps/courses/views.pyhttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-0(30)
• M karmaworld/assets/css/global.csshttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-1(19)
• M karmaworld/assets/js/add-course.jshttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-2(54)
• M karmaworld/assets/js/course-detail.jshttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-3(65)
• A karmaworld/assets/js/course.jshttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-4(58)
• M karmaworld/templates/courses/course_detail.htmlhttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-5(95)
• M karmaworld/templates/partial/add_course.htmlhttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-6(5)
• M karmaworld/urls.pyhttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-7(4)
• M karmaworld/utils/ajax_increment.pyhttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-8(1)

Patch Links:

• https://github.com/FinalsClub/karmaworld/pull/314.patch
• https://github.com/FinalsClub/karmaworld/pull/314.diff

Reply to this email directly or view it on GitHubhttps://github.com/FinalsClub/karmaworld/pull/314 .

[btbonval]

It's called versioning, although upsert can assist that. http://stackoverflow.com/a/3772970

Something like this would work. I haven't done research, so this is just a for-example not a recommendation: https://www.djangopackages.com/grids/g/versioning/

On Sat, Feb 1, 2014 at 5:05 PM, Bryan btbonval@gmail.com wrote:

Hrm, maybe UPSERT isn't the right word, because wikipedia basically says it is nothing but the usual create or update.

In the parlance I've used with other database devs, UPSERT treats UPDATEs as INSERTs. That is to say, if you update record A, then instead of updating A, you create a new record B that contains what A would have been modified to. Somehow B is marked as newer or more correct than A. DELETEs may be similarly handled with some kind of INSERT relevant to record A which marks a flag saying that A is deleted. Usually the tables contain logs of <timestamp, primary key> together as the composite primary key, and instead of looking for record A, you would look for the max(timestamp) on all A-like records in the log table.

This is done in any system where destructive UPDATEs/DELETEs must be avoided. It's a recovery mechanism of sorts. DELETE and UPDATE are unrecoverable. When you open the site to anonymous DELETE and anonymous UPDATE (or many other reasons), you really don't want adversaries to be able to destroy data that was previously there in an unrecoverable way. Backups are great, but even they aren't an adequate answer unless the capability exists to check 100% of the database prior to the periodic rollover that backups experience.

Think of it like Wikipedia itself. You don't simply UPDATE a Wikipedia page when you edit it. Although you're editing it, the database gets an INSERT. Look at the history log on Wikipedia and you'll see exactly what I mean when I say UPSERT. Timestamped entries that can be viewed historically or reverted to as needed when things get icky.

Anyway, you shouldn't need to implement functionality like that yourself. It should already exist somewhere. Unfortunately it looks like UPSERT might not be the right name, at least not according to wikipedia's brief high level summary. I know Rails this feature built in. I should find what they call it and do a search for that against Django. -Bryan

On Sat, Feb 1, 2014 at 4:52 PM, Jacob Hilker notifications@github.comwrote:

Quick preliminary PR for ticket #208https://github.com/FinalsClub/karmaworld/issues/208

Since we have no examples of doing a AJAX edit in the application, and I have no experience with django before this, I took a stab at what I thought might be best practices (after a few iterations). Looking for any feedback.

Ticket still needs: UPSERT - The definition of UPSERT that I know is that it is simply an update or insert call, and either updates an object if found, or creates it. It sounds like what we want however is versioning, where the latest version is what is used.

Course name slug - Should this slug be modified from the original if the course name changes? Con - current page url would be broken, people bookmarks might break. Pro - url properly represents course. I'm assuming we should, but figured I'd double check

Icon - I need to make an edit icon, should be simple enough.

Course Name - School uniqueness validation. It appears we have no validator for this (it breaks on add too) We should probably have one in a general place, haven't looked into doing this yet.

First comment regarding note tag display + edit - Should probably be it's own ticket.

Requirement to be able to do this - Email regarding privileges states that editing the course meta data should require karma. I assume this means

login as well. How much karma should be required to edit courses?

You can merge this Pull Request by running

git pull https://github.com/FinalsClub/karmaworld 208-edit-course-properties-main

Or view, comment on, or merge it at:

https://github.com/FinalsClub/karmaworld/pull/314 Commit Summary

• 208 - Edit course properties - first pass

File Changes

• M karmaworld/apps/courses/views.pyhttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-0(30)
• M karmaworld/assets/css/global.csshttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-1(19)
• M karmaworld/assets/js/add-course.jshttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-2(54)
• M karmaworld/assets/js/course-detail.jshttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-3(65)
• A karmaworld/assets/js/course.jshttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-4(58)
• M karmaworld/templates/courses/course_detail.htmlhttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-5(95)
• M karmaworld/templates/partial/add_course.htmlhttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-6(5)
• M karmaworld/urls.pyhttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-7(4)
• M karmaworld/utils/ajax_increment.pyhttps://github.com/FinalsClub/karmaworld/pull/314/files#diff-8(1)

Patch Links:

• https://github.com/FinalsClub/karmaworld/pull/314.patch
• https://github.com/FinalsClub/karmaworld/pull/314.diff

Reply to this email directly or view it on GitHubhttps://github.com/FinalsClub/karmaworld/pull/314 .

[JHilker]

I've decided to use https://github.com/etianen/django-reversion

Overall it seems very simple and clean, and later on we can add an admin panel so we don't need to use the shell to access revisions.

Requirements are only that we specify where we create revisions, I've added them on the initial save, my edit, and the note update. This also catches the flag being added. Do we want revisions at all of these points?

You will need to run ./manage.py syncdb when merged. (I also had to run a ./manage.py migrate revision, but this appears to not be the standard)

[btbonval]

Based on your commits, reversion appears to be method-level and not class-level.

Yet, quickly glancing through the docs, it appears to be one-line per model: http://django-reversion.readthedocs.org/en/latest/how-it-works.html

Just so I can get a feel for what you did, what instructions were you following,?

[JHilker]

I was looking at http://django-reversion.readthedocs.org/en/latest/api.html, which looking back may not be correct. However removing the method level lines led to no revisions being created. Trying the middleware now.

[JHilker]

You were right, I took a more complicated approach. Toss in the middleware instead and everything works automagically. I like.

[btbonval]

I don't see this bit of code anywhere:

import reversion

reversion.register(YourModel)

http://django-reversion.readthedocs.org/en/latest/how-it-works.html#saving-revisions

Does adding it to the middleware add revisioning to all classes? That might be overkill. I haven't actually read the docs thoroughly. It looks like the middleware is a necessary but not sufficient to enable revisions. I could be wrong. http://django-reversion.readthedocs.org/en/latest/how-it-works.html#revision-management

[charlesconnell]

The UI for editing is nice, seems to work just as one would expect. Great that we are using the same form for creating a course. The "reversioning" does not seem to be working.