-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/598 comment htmlpage thread contents endpoints #91
Feature/598 comment htmlpage thread contents endpoints #91
Conversation
Pull Request Test Coverage Report for Build 264
💛 - Coveralls |
Pull Request Test Coverage Report for Build 305
💛 - Coveralls |
…r instead of contributor for few view)
This reverts commit e2edf42.
@@ -152,6 +152,13 @@ For example, with default config: | |||
# launch your favorite web-browser | |||
firefox http://localhost:6543/api/v2/doc/ | |||
|
|||
## Roles, profile and access rights | |||
|
|||
In Tracim, only some user can access to some informations, this is also true in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is also true in Tracim REST API
=> this is checked at REST API layer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also be checked in webdav layer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, it MUST be check in webdav layer too. It's implemented or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should check this better, but i do think there is actually only restriction on visible workspaces according to user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tracim/fixtures/content.py
Outdated
) | ||
bob_content_api.create_comment( | ||
parent=best_cake_thread, | ||
content='<p>What about Apple Pie ? There are Awesome !</p>', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in English, there's no space before punctuation : What about Apple Pie ? There are Awesome !
=> What about Apple Pie? There are Awesome!
) | ||
reader_content_api.create_comment( | ||
parent=best_cake_thread, | ||
content='<p>You are right, but Kouign-amann are clearly better.</p>', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not understand this one, content selection is maybe uncorrect .
tracim/fixtures/content.py
Outdated
): | ||
bob_content_api.update_content( | ||
item=best_cake_thread, | ||
new_content='What is the best cake ?', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same remark
tracim/lib/core/content.py
Outdated
@@ -161,6 +164,10 @@ def show( | |||
def get_content_in_context(self, content: Content): | |||
return ContentInContext(content, self._session, self._config) | |||
|
|||
def get_revision_in_context(self, revision: ContentRevisionRO): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing typing
author = marshmallow.fields.Nested(UserDigestSchema) | ||
|
||
|
||
class ThreadRevisionSchema(RevisionSchema): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same remark. Define a unique TextBasedContentRevisionSchema
used in place of ThreadRevisionSchema
and HtmlDocumentRevisionSchema
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
#### | ||
|
||
class CommentSchema(marshmallow.Schema): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CommentSchema can be overwritten with TextBasedContentSchema, doesnt it ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably, tracim/tracim#618
) | ||
|
||
|
||
class ContentModifySchema(marshmallow.Schema): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the schema is not intended to be used as is, maybe rename it to something more clear. ContentModifyAbstractSchema
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be done here : tracim/tracim#618
) | ||
|
||
|
||
class HtmlDocumentModifySchema(ContentModifySchema): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This schema is to rename to TextBasedContentModifySchema
and to use in place of HTMLDocumentUpdate
and ThreadUpdate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be done in same time as tracim/tracim#618
|
||
@hapic.with_api_doc(tags=[COMMENT_ENDPOINTS_TAG]) | ||
@hapic.handle_exception(NotAuthenticated, HTTPStatus.UNAUTHORIZED) | ||
@hapic.handle_exception(InsufficientUserWorkspaceRole, HTTPStatus.FORBIDDEN) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InsufficientUserWorkspaceRole
is probably to rename to InsufficientUserRoleInWorkspace
. The reason for that is "it's understandable as naturally read"
require #88
Bug/Missing:
Others:
closes tracim/tracim#598
closes tracim/tracim#601
closes tracim/tracim#606
closes tracim/tracim#597