Skip to content
This repository was archived by the owner on Mar 25, 2025. It is now read-only.

Hot - Fix for SuspeciousFileOperation error on certificate preview. - #308

Merged
tehreem-sadat merged 1 commit into
developfrom
tehreem/fix_preview_certificate_error
Feb 24, 2020
Merged

Hot - Fix for SuspeciousFileOperation error on certificate preview.#308
tehreem-sadat merged 1 commit into
developfrom
tehreem/fix_preview_certificate_error

Conversation

@tehreem-sadat

@tehreem-sadat tehreem-sadat commented Feb 21, 2020

Copy link
Copy Markdown

Story Link

244

PR Description

An error was due to extra starting "/" in the signatory image path. For now, I have removed that "/".

Type of change

Please select the options that are relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Change

How to test?

Steps:

  • Create a course certificate from course settings dropdown on the studio.
  • Click on the "preview certificate" button from the top right corner.

Result:
The certificate should be displayed without any error.

Checklist before merging:

  • Squased
  • Reviewd

Comment thread lms/djangoapps/certificates/api.py Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we should first verify that signature_image_path is starting with a / and only then truncate it. But if this is for the hotfix purposes only, then can be ignored for now I guess.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I agree we should care about an edge case here, otherwise, it can raise an exception.

@tehreem-sadat
tehreem-sadat force-pushed the tehreem/fix_preview_certificate_error branch from fdaf1b7 to b6fe96e Compare February 24, 2020 07:18
Comment thread lms/djangoapps/certificates/api.py Outdated
Comment on lines 481 to 483

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we don't need this part, do we?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

(y)

@tehreem-sadat
tehreem-sadat force-pushed the tehreem/fix_preview_certificate_error branch from b6fe96e to e459f3c Compare February 24, 2020 08:01
@tehreem-sadat
tehreem-sadat merged commit 0ca25e0 into develop Feb 24, 2020
@tehreem-sadat
tehreem-sadat deleted the tehreem/fix_preview_certificate_error branch February 24, 2020 09:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants