FinalsClub / karmaworld

KarmaNotes.org v3.0
GNU Affero General Public License v3.0
7 stars 6 forks source link

edit notes with no category? #403

Closed btbonval closed 9 years ago

btbonval commented 9 years ago

Logged in with an admin account.

Currently testing against this page: /note/Brown/management-of-industrial-and-nonprofit-organizations/this-is-a-test-of-evernote

There is markdown and HTML in the database. The page loads fine. The note looks like any other note. I can highlight and add annotations, but I don't see any WYSIWYG interface.

Clicking "Edit this Note" brings up name and tags for editing, but nothing else. No WYSIWYG interface.

btbonval commented 9 years ago

No errors on JS console.

btbonval commented 9 years ago

WYSIHTML is being downloaded according to network tab. All these are downloaded (no longer inside compress):

btbonval commented 9 years ago

@yourcelf Any special way to bring up the interface?

btbonval commented 9 years ago

running repo with latest commit to fix 500 errors but otherwise no changes.

 $ git status
On branch note-editing
Untracked files:
  (use "git add <file>..." to include in what will be committed)

    .vagrant/
    Vagrantfile

nothing added to commit but untracked files present (use "git add" to track)
$ git log
commit 14fb88539d22436efdf81bfdd9c0acd9af16000f
Author: Bryan <btbonval@gmail.com>
Date:   Wed Feb 18 16:15:07 2015 -0500

    pulling wysihtml5 out of compress to stop key errors for #397

commit 4736b4b0994b07623ed5562090ddec9c8b3e2f4e
Author: Charlie DeTar <cfd@media.mit.edu>
Date:   Sat Feb 14 16:27:17 2015 -0700

    Updates to sanitizer; all-in with inline HTML
btbonval commented 9 years ago

This looks like the code that should be turning on the buttons. https://github.com/FinalsClub/karmaworld/blob/note-editing/karmaworld/templates/notes/note_base.html#L67-L83

btbonval commented 9 years ago

Ran through the above code in my console, replacing var scope = $(event.currentTarget); with var scope = $('#note-edit-dialog') (which was open and highlighted by the inspector at the time).

The second to last .show() line changed nothing on the UI.

The very last line, initWysihtml5(textareaClone[0]); errored with:

TypeError: element is null
btbonval commented 9 years ago

Ah weird. Searching the source code for the page I'm looking at (http://127.0.0.1:8080/note/Brown/management-of-industrial-and-nonprofit-organizations/this-is-a-test-of-evernote), I cannot find var scope or sandbox anywhere in the source. However, that should be part of the code generated by note_base.

btbonval commented 9 years ago

{% if note.is_editable %}

sigh.

k.

btbonval commented 9 years ago

Looks like is_editable is based directly on category, the field never made it into the fray.

There is an empty category for the note I'm presently testing.

Revising ticket to ask if notes without category should be editable or not.

yourcelf commented 9 years ago

Good question. Indeed, after starting to go down that road, I just decided to stick to the method on the model, which we can update to use a property on the backend or not. At the moment, with no immediate plans to have editability determined by anything, it seems to make sense to me to just make it a method and migrate to a field in the future if that's a better solution.

Would be easy to update the method to return true if the category is in editable categories or is None.

btbonval commented 9 years ago

It's worth checking production to see how many notes have no category. This might be a non-issue.

yourcelf commented 9 years ago

One other note: there is complexity around initing the WYSIHTML editor due to it being in a modal. In short, the dom needs to be visible before the editor is attached; but the editor doesn't provide a way to tear itself down. So to support arbitrary change-on-open, the implementation I did manually tears down the editor every time the modal closes, and reinstantiates it every time the modal opens.

btbonval commented 9 years ago

I saw that reading the code. It's a little rough but it was well commented and I understood what you were doing.

I was perplexed because the DOM wasn't there in the first place, but of course I found that template if check later ;)

btbonval commented 9 years ago

PS checked against /note/harvard/the-road-to-postmodernism-149/april-11doc and for the first time I have gotten the edit interface on the prod-like VM! hooray! I edited the note, saved it, and it all worked!

btbonval commented 9 years ago

Checking production's database for the category situation.

btbonval commented 9 years ago

Junk. Definitely notes without category on prod. In fact, it's the overwhelming majority.

=> SELECT distinct(category) FROM notes_note;
   category    
---------------

 STUDY_GUIDE
 LECTURE_NOTES
 OTHER
(4 rows)

=> SELECT count(*) FROM notes_note WHERE LENGTH(COALESCE(category,'')) = 0;
 count 
-------
  4732
(1 row)

=> SELECT count(*) FROM notes_note;
 count 
-------
  5010
(1 row)

@AndrewMagliozzi What do you want to do about notes without a category? Should they be editable? There is no interface to add a category to a note that lacks one, even as an admin (I've checked), except through the Django Admin interface.

btbonval commented 9 years ago

Alright I'm going to make an executive decision on this one.

Notes without any category cannot be edited.

The resolution will be from closing out #371 , wherein the category of notes can be changed.