-
Notifications
You must be signed in to change notification settings - Fork 55
Use jira client token for media #1335
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
base: main
Are you sure you want to change the base?
Conversation
9620015 to
347aecb
Compare
| viewMediaClientConfig: { | ||
| authProvider: () => | ||
| Promise.resolve({ | ||
| token: '', |
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.
🔥 Code Bugs
The hardcoded empty token and clientId in the media auth provider will prevent media functionality from working properly.
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.
Yeap, this should be replaced by real call to Jira API to get a token.
| ), | ||
| resolve(JSON.stringify(document)); | ||
| }, | ||
| // { |
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.
🔎 Code Readability
The commented-out transformer code should be removed rather than left as dead code.
| } | ||
|
|
||
| // TODO: Temporary set api version to 3 to get ADF value instead of string(WikiMarkup). At the time we have access to token endpoint v3 should be default for cloud. | ||
| (newClient as any).apiVersion = '3'; |
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.
⚠️ Maintainability - Best Practices
The type cast (newClient as any).apiVersion = '3' bypasses TypeScript safety and could break if the client structure changes.
| setCommentText(''); | ||
| closeEditorHandler(); | ||
| } | ||
| // if (content && content.trim() !== '') { |
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.
🔎 Code Readability
The commented validation logic should be removed entirely rather than left as commented code.
What Is This Change?
How Has This Been Tested?
Basic checks:
npm run lintnpm run testAdvanced checks:
Recommendations: