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

Adding notes #32

Merged
merged 23 commits into from
Nov 30, 2013
Merged

Adding notes #32

merged 23 commits into from
Nov 30, 2013

Conversation

ukanga
Copy link
Member

@ukanga ukanga commented Nov 28, 2013

Adding notes to a submission via api and via web form on instance view.

@ukanga
Copy link
Member Author

ukanga commented Nov 28, 2013

@pld note that export of notes is not implemented yet, not as straight forward I had initially thought.

@@ -4,3 +4,4 @@
from xform import XForm
from odk_logger.xform_instance_parser import InstanceParseError
from ziggy_instance import ZiggyInstance
from .note import Note
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest we use the full namespace here to keep it consistent with the other imports, and to make it clearer to devs

added KoBo to license
@ukanga
Copy link
Member Author

ukanga commented Nov 30, 2013

@pld made changes, please have a look

from odk_viewer.models import ParsedInstance

from api.models import Project, OrganizationProfile, ProjectXForm, Team
from api.forms import NoteForm
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see this used anywhere in this file, is the import needed, is the api/forms.py file needed? Am I missing something about django?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I first shifted the form to a separate file as suggested earlier, then when I started using the NoteSerializer I no longer had the need to use the form. So this should be removed, I will do that.

@pld
Copy link
Member

pld commented Nov 30, 2013

@ukanga this looks good, just a question about the form file. are exports in here?

pld pushed a commit that referenced this pull request Nov 30, 2013
Adding notes, green, merge to keep things in sync
@pld pld merged commit 3a8dacc into onaio:master Nov 30, 2013
@pld pld deleted the adding-notes branch November 30, 2013 13:59
@ukanga
Copy link
Member Author

ukanga commented Dec 2, 2013

@pld exports are also covered in this pull request, a conversation with @larryweya made this way simler to implement 1fd1a00

@pld
Copy link
Member

pld commented Dec 2, 2013

Sound great, I merged the previous PR, is there a new one for removing that
class?
On Dec 2, 2013 12:08 AM, "ukanga" notifications@github.com wrote:

@pld https://github.com/pld exports are also covered in this pull
request, a conversation with @larryweya https://github.com/larryweyamade this way simler to implement
1fd1a00 1fd1a00


Reply to this email directly or view it on GitHubhttps://github.com//pull/32#issuecomment-29594914
.

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

Successfully merging this pull request may close these issues.

3 participants