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

Certificate Generation #785

Merged
merged 19 commits into from
Nov 29, 2022
Merged

Certificate Generation #785

merged 19 commits into from
Nov 29, 2022

Conversation

itsomkathe
Copy link
Contributor

@itsomkathe itsomkathe commented Nov 3, 2022

Refer to issue #755

People to loop in / review from
@komalahire @jschanker

@itsomkathe itsomkathe added the enhancement New feature or request label Nov 3, 2022
@itsomkathe itsomkathe self-assigned this Nov 3, 2022
@vercel
Copy link

vercel bot commented Nov 3, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
bhanwari-devi ✅ Ready (Inspect) Visit Preview Nov 25, 2022 at 8:20AM (UTC)
bhanwari-devi-old ✅ Ready (Inspect) Visit Preview Nov 25, 2022 at 8:20AM (UTC)
1 Ignored Deployment
Name Status Preview Updated
meraki-dev ⬜️ Ignored (Inspect) Nov 25, 2022 at 8:20AM (UTC)

Copy link
Collaborator

@jschanker jschanker left a comment

Choose a reason for hiding this comment

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

Initial comments

if(!openModal){
axios({
method: METHODS.GET,
url: `${process.env.REACT_APP_MERAKI_URL}/certificate`,
Copy link
Collaborator

@jschanker jschanker Nov 3, 2022

Choose a reason for hiding this comment

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

@kirithiv2000 Making the API call directly as follows (removing the token):

fetch('https://api.merakilearn.org/certificate', {headers: {"Authorization": 'xxx', "accept": "application/json"}}).then(res => console.log(res));

is resulting in an Internal Server Error for me. Took a quick look and this line here: https://github.com/navgurukul/sansaar/blob/dev-copy/lib/services/generateCertificate.js#L86
will cause an error (can't call toString on null) and isn't used so it can be removed

Copy link
Collaborator

@jschanker jschanker Nov 3, 2022

Choose a reason for hiding this comment

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

Realized that was probably deliberate to force an error to catch since it was in a try block. But it crashes for me when a direct request is made.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have you marked the course as complete through API?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you marked the course as complete through API?

No, so the request should produce an error response here, I just wasn't sure if it should be a generic 500 Internal Server Error.

src/components/PathwayCourse/index.js Outdated Show resolved Hide resolved
src/components/PathwayCourse/index.js Outdated Show resolved Hide resolved
src/components/PathwayCourse/index.js Outdated Show resolved Hide resolved
src/components/PathwayCourse/index.js Outdated Show resolved Hide resolved
src/components/PathwayCourse/index.js Outdated Show resolved Hide resolved
src/components/PathwayCourse/styles.js Outdated Show resolved Hide resolved
src/components/PathwayCourse/styles.js Show resolved Hide resolved
src/components/PathwayCourse/index.js Outdated Show resolved Hide resolved
src/components/PathwayCourse/index.js Outdated Show resolved Hide resolved
@itsomkathe
Copy link
Contributor Author

The react-pdf package that we are using to show PDF as an image is having some issues, which are not solved by the maintainers yet. So as of now, this solution cannot be used.

You can read this thread: wojtekmaj/react-pdf#1043

Below is the screenshot attached

Capture

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

Successfully merging this pull request may close these issues.

Certificate Generation
3 participants