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

Feature/16 save extra report notes #23

Closed

Conversation

EmiM
Copy link

@EmiM EmiM commented Oct 5, 2020

Added saving report extra notes via api ( #16 )
Not sure if this is the refactoring you had in mind. Let me know if I should add some tests.

To be merged after #22

Copy link
Member

@ivellios ivellios left a comment

Choose a reason for hiding this comment

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

Appreciate your contribution. Thank you :)

Copy link
Member

@ivellios ivellios left a comment

Choose a reason for hiding this comment

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

Found one issue and left a note

@@ -45,6 +45,7 @@ class Meta:
"date",
"table_name",
"table_extra_notes",
"report_notes",
Copy link
Member

Choose a reason for hiding this comment

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

This does not fully cover requirement on who can see the report.

Anyone with access to this endpoint can pull the data and read the notes. It should be returned as a null or empty string, if the requesting user is not a DM. So we need a backend to check access to the field. This can be easily covered with MethodField.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the info. I added check in the serializer.

@ivellios
Copy link
Member

btw. You can change description and add:
closes #15
closes #16

-- it will close the issues automatically upon merge and close :)

@@ -64,6 +65,11 @@ def get_time_start(self, game):
def get_time_end(self, game):
return game.time_end.strftime("%H:%M") if game.time_end else ""

def get_report_notes(self, game):
if self.context["request"].user.profile != game.dm:
Copy link
Member

Choose a reason for hiding this comment

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

Actually it should be visible to ANY DM in the system, so the role of the profile should be checked. AFAIR there's a method/property for that.

@ivellios ivellios closed this Oct 4, 2024
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.

2 participants