-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add support of version masking #1393
Conversation
UI PR at c2corg/c2c_ui#2947 |
schema, | ||
cook_locale=True | ||
) | ||
# TODO if masked, return limited info as in DocumentInfoRest? |
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 TODO is not yet resolved?
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.
No, it's not: as for now when a version is masked, the API returns nothing as the archived documents. See https://github.com/c2corg/v6_api/pull/1393/files#diff-33ce4bfbec662d496fcd00822bfdb6174d23586982306f66c15194dba40f771cR72
Perhaps returning at least the version title as in DocumentInfoRest
would be nice? See
v6_api/c2corg_api/views/document_info.py
Lines 17 to 20 in 969bf13
class DocumentInfoRest(object): | |
""" Base class for all views that return a basic/info version for a | |
document, that only contains the document_id and title/title_prefix | |
of the requested locale. |
I have not tried yet to figure out if this is easily feasible though.
There's still an issue as the cache of the versions and of the document history is not refreshed when a version is masked/unmasked. |
7cf8ee7
to
62964d1
Compare
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.
(Stupid?) remarks / questions, please keep in mind I'm dumb as f*** with python:
- you're not testing that only a moderator can read a version that is masked?
- what about diffs? If we can make the diff between a masked version and another, then it's the same as if we didn't mask it.
As far as I invstigated it some time ago, a moderator will see the diff between all versions, but a normal user will se the diff between unmasked versions only. |
@lbesson A version is returned only if not masked or if the user is a moderator. See About the diff: there is no diff view on the API side. It's done on the UI side making requests to the API to get both versions to diff. If one version is masked it won't be exposed on the UI side either when diffing. |
Yes. I was just saying there are no unit tests added to check that this is properly implemented. |
So we're back to the 'stupid' remark qualification :) |
should we merge? |
@lbesson I had planned to do so when the UI part would be ready but I think we can merge it anyway as the PR only adds a new attribute and new services/tests, not changing the existing API behaviour. |
How is db migration handled? I understand alembic is used for that, but should the migration scripts be run manually, or is the service handling it? |
https://github.com/c2corg/v6_api/wiki/Upgrade-the-prod-server#full-how-to |
Thanks @brunobesson for the link to the doc. |
@lbesson @brunobesson I will merge the PR this evening if it's OK for you. |
No description provided.