Sentry plugin refactoring - code review changes

This commit is contained in:
Wojciech Adaszynski
2020-05-20 09:57:32 +02:00
parent 0783e07290
commit baf8188db7
17 changed files with 73 additions and 85 deletions
+1 -1
View File
@@ -15,7 +15,7 @@
"@backstage/plugin-tech-radar": "^0.1.1-alpha.6",
"@backstage/plugin-welcome": "^0.1.1-alpha.6",
"@backstage/theme": "^0.1.1-alpha.6",
"@backstage/plugin-sentry": "^0.1.1-alpha.5",
"@backstage/plugin-sentry": "^0.1.1-alpha.6",
"@material-ui/core": "^4.9.1",
"@material-ui/icons": "^4.9.1",
"@material-ui/lab": "4.0.0-alpha.45",
+1 -1
View File
@@ -22,7 +22,7 @@
"@material-ui/core": "^4.9.1",
"@material-ui/icons": "^4.9.1",
"@material-ui/lab": "4.0.0-alpha.45",
"@backstage/plugin-sentry": "^0.1.1-alpha.5",
"@backstage/plugin-sentry": "^0.1.1-alpha.6",
"react": "^16.13.1",
"react-dom": "^16.13.1",
"react-use": "^14.2.0"
+5 -5
View File
@@ -1,6 +1,6 @@
{
"name": "@backstage/plugin-sentry-backend",
"version": "0.1.1-alpha.5",
"version": "0.1.1-alpha.6",
"main": "dist",
"types": "src/index.ts",
"license": "Apache-2.0",
@@ -13,8 +13,8 @@
"clean": "backstage-cli clean"
},
"dependencies": {
"@backstage/backend-common": "^0.1.1-alpha.5",
"@backstage/core": "^0.1.1-alpha.5",
"@backstage/backend-common": "^0.1.1-alpha.6",
"@backstage/core": "^0.1.1-alpha.6",
"axios": "^0.19.2",
"cors": "^2.8.5",
"express": "^4.17.1",
@@ -23,8 +23,8 @@
"winston": "^3.2.1"
},
"devDependencies": {
"@backstage/cli": "^0.1.1-alpha.5",
"@backstage/dev-utils": "^0.1.1-alpha.5",
"@backstage/cli": "^0.1.1-alpha.6",
"@backstage/dev-utils": "^0.1.1-alpha.6",
"@types/jest": "^25.2.1",
"@types/node": "^12.0.0"
},
+8 -14
View File
@@ -16,25 +16,19 @@
import { Logger } from 'winston';
import Router from 'express-promise-router';
import express from 'express';
import { SentryApiForwarder } from './sentry-api';
import { getSentryApiForwarder } from './sentry-api';
export async function createRouter(
rootLogger: Logger,
): Promise<express.Router> {
export async function createRouter(logger: Logger): Promise<express.Router> {
const router = Router();
const SENTRY_TOKEN = process.env.SENTRY_TOKEN;
if (!SENTRY_TOKEN) {
console.error('Sentry token must be provided in env to start the API.');
process.exit(1);
throw new Error(
'Sentry token must be provided in SENTRY_TOKEN environment variable to start the API.',
);
}
const sentryForwarder = new SentryApiForwarder(SENTRY_TOKEN);
const logger = rootLogger.child({ plugin: 'sentry' });
const sentryForwarder = getSentryApiForwarder(SENTRY_TOKEN, logger);
router.get('*', (req, res) => sentryForwarder.fowardRequest(req, res));
router.use(sentryForwarder);
const app = express();
app.set('logger', logger);
app.use('/', router);
return app;
return router;
}
@@ -13,13 +13,11 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { SentryApiForwarder } from './sentry-api';
import { getRequestHeaders } from './sentry-api';
describe('SentryApiForwarder', () => {
it('should generate headers based on token passed in constructor', () => {
const forwarder = new SentryApiForwarder('testtoken');
expect(forwarder.getRequestHeaders()).toEqual({
expect(getRequestHeaders('testtoken')).toEqual({
headers: {
Authorization: `Bearer testtoken`,
},
@@ -15,29 +15,33 @@
*/
import express from 'express';
import axios from 'axios';
import { Logger } from 'winston';
export class SentryApiForwarder {
constructor(private token: string) {}
export function getRequestHeaders(token: string) {
return {
headers: {
Authorization: `Bearer ${token}`,
},
};
}
// public for testing
public getRequestHeaders() {
return {
headers: {
Authorization: `Bearer ${this.token}`,
},
};
}
public fowardRequest(request: express.Request, response: express.Response) {
export function getSentryApiForwarder(token: string, logger: Logger) {
return function fowardRequest(
request: express.Request,
response: express.Response,
) {
const sentryUrl = request.path;
const effectiveUrl = `https://sentry.io/${sentryUrl}`;
logger.info(`Calling Sentry REST API, ${effectiveUrl}`);
axios
.get(`https://sentry.io/${sentryUrl}`, this.getRequestHeaders())
.then((res) => {
.get(effectiveUrl, getRequestHeaders(token))
.then(res => {
response.send(res.data);
})
.catch((err) => {
.catch(err => {
return response.status(err.response.status).json({
detail: err.response.statusText,
});
});
}
};
}
-11
View File
@@ -1,11 +0,0 @@
{
"extends": "../../packages/backend/tsconfig.json",
"include": [
"./src"
],
"compilerOptions": {
"baseUrl": "./src",
"outDir": "./dist",
"skipLibCheck": true
}
}
+9 -8
View File
@@ -1,7 +1,8 @@
{
"name": "@backstage/plugin-sentry",
"version": "0.1.1-alpha.5",
"version": "0.1.1-alpha.6",
"main": "dist/index.esm.js",
"main:src": "src/index.ts",
"types": "src/index.ts",
"license": "Apache-2.0",
"private": true,
@@ -16,8 +17,8 @@
"clean": "backstage-cli clean"
},
"dependencies": {
"@backstage/core": "^0.1.1-alpha.5",
"@backstage/theme": "^0.1.1-alpha.5",
"@backstage/core": "^0.1.1-alpha.6",
"@backstage/theme": "^0.1.1-alpha.6",
"@material-ui/core": "^4.9.1",
"@material-ui/icons": "^4.9.1",
"@material-ui/lab": "4.0.0-alpha.45",
@@ -28,12 +29,12 @@
"timeago.js": "^4.0.2"
},
"devDependencies": {
"@backstage/cli": "^0.1.1-alpha.5",
"@backstage/dev-utils": "^0.1.1-alpha.5",
"@testing-library/jest-dom": "^4.2.4",
"@backstage/cli": "^0.1.1-alpha.6",
"@backstage/dev-utils": "^0.1.1-alpha.6",
"@testing-library/jest-dom": "^5.7.0",
"@testing-library/react": "^9.3.2",
"@testing-library/user-event": "^7.1.2",
"@types/jest": "^25.2.1",
"@testing-library/user-event": "^10.2.4",
"@types/jest": "^25.2.2",
"@types/node": "^12.0.0",
"@types/testing-library__jest-dom": "^5.0.4",
"jest-fetch-mock": "^3.0.3"
@@ -39,12 +39,12 @@ describe('SentryIssuesTable', () => {
<SentryIssuesTable sentryIssues={issues} />
</ThemeProvider>,
);
expect(await table.findByText('Error')).toBeInTheDOM();
expect(await table.findByText('Graph')).toBeInTheDOM();
expect(await table.findByText('First seen')).toBeInTheDOM();
expect(await table.findByText('Last seen')).toBeInTheDOM();
expect(await table.findByText('Events')).toBeInTheDOM();
expect(await table.findByText('Users')).toBeInTheDOM();
expect(await table.findByText('Error')).toBeInTheDocument();
expect(await table.findByText('Graph')).toBeInTheDocument();
expect(await table.findByText('First seen')).toBeInTheDocument();
expect(await table.findByText('Last seen')).toBeInTheDocument();
expect(await table.findByText('Events')).toBeInTheDocument();
expect(await table.findByText('Users')).toBeInTheDocument();
});
it('should render values in a table', async () => {
const issues: SentryIssue[] = [
@@ -63,9 +63,9 @@ describe('SentryIssuesTable', () => {
<SentryIssuesTable sentryIssues={issues} />
</ThemeProvider>,
);
expect(await table.findByText('Exception')).toBeInTheDOM();
expect(await table.findByText('exception was thrown')).toBeInTheDOM();
expect(await table.findByText('101')).toBeInTheDOM();
expect(await table.findByText('202')).toBeInTheDOM();
expect(await table.findByText('Exception')).toBeInTheDocument();
expect(await table.findByText('exception was thrown')).toBeInTheDocument();
expect(await table.findByText('101')).toBeInTheDocument();
expect(await table.findByText('202')).toBeInTheDocument();
});
});
@@ -26,7 +26,7 @@ const errorApi = { post: () => {} };
describe('SentryPluginPage', () => {
it('should render header and time switched', () => {
mockFetch.mockResponse(() => new Promise(() => {}));
mockFetch.mockResponse('{}');
const rendered = render(
<ApiProvider apis={ApiRegistry.from([[errorApiRef, errorApi]])}>
<ThemeProvider theme={lightTheme}>
@@ -24,13 +24,13 @@ import {
ContentHeader,
SupportButton,
} from '@backstage/core';
import SentryPluginWidget from '../SentryPluginWidget/SentryPluginWidget';
import { SentryPluginWidget } from '../SentryPluginWidget/SentryPluginWidget';
import { ToggleButton, ToggleButtonGroup } from '@material-ui/lab';
const SentryPluginPage: FC<{}> = () => {
const [statsFor, setStatsFor] = useState<'12h' | '24h'>('12h');
const toggleStatsFor = () =>
statsFor === '12h' ? setStatsFor('24h') : setStatsFor('12h');
const toggleStatsFor = () => setStatsFor(statsFor === '12h' ? '12h' : '24h');
return (
<Page theme={pageTheme.tool}>
<Header title="Welcome to Sentry Plugin!">
@@ -28,7 +28,7 @@ import { sentryApiFactory } from '../../data/api-factory';
const api = sentryApiFactory('spotify');
const SentryPluginWidget: FC<{
export const SentryPluginWidget: FC<{
sentryProjectId: string;
statsFor: '24h' | '12h';
}> = ({ sentryProjectId, statsFor }) => {
@@ -55,5 +55,3 @@ const SentryPluginWidget: FC<{
return <SentryIssuesTable sentryIssues={value || []} />;
};
export default SentryPluginWidget;
+2 -1
View File
@@ -16,7 +16,8 @@
import { SentryIssue } from './sentry-issue';
import { SentryApi } from './sentry-api';
const API_BASE_URL = 'http://localhost:7000/sentry/api/0/projects/';
const API_HOST = process.env.API_HOST || 'http://localhost:7000';
const API_BASE_URL = `${API_HOST}/sentry/api/0/projects/`;
export class ProductionSentryApi implements SentryApi {
private organization: string;
+1 -1
View File
@@ -15,4 +15,4 @@
*/
export { plugin } from './plugin';
export { default as SentryIssuesWidget } from './components/SentryPluginWidget/SentryPluginWidget';
export { SentryPluginWidget as SentryIssuesWidget } from './components/SentryPluginWidget/SentryPluginWidget';
+1 -1
View File
@@ -14,5 +14,5 @@
* limitations under the License.
*/
import '@testing-library/jest-dom/extend-expect';
import '@testing-library/jest-dom';
require('jest-fetch-mock').enableMocks();
-5
View File
@@ -1,5 +0,0 @@
{
"extends": "../../tsconfig.json",
"include": ["src", "dev"],
"compilerOptions": {}
}
+9 -1
View File
@@ -4015,6 +4015,14 @@
jest-diff "^25.2.1"
pretty-format "^25.2.1"
"@types/jest@^25.2.1":
version "25.2.3"
resolved "https://registry.npmjs.org/@types/jest/-/jest-25.2.3.tgz#33d27e4c4716caae4eced355097a47ad363fdcaf"
integrity sha512-JXc1nK/tXHiDhV55dvfzqtmP4S3sy3T3ouV2tkViZgxY/zeUkcpQcQPGRlgF4KmWzWW5oiWYSZwtCB+2RsE4Fw==
dependencies:
jest-diff "^25.2.1"
pretty-format "^25.2.1"
"@types/jest@^25.2.2":
version "25.2.2"
resolved "https://registry.npmjs.org/@types/jest/-/jest-25.2.2.tgz#6a752e7a00f69c3e790ea00c345029d5cefa92bf"
@@ -5322,7 +5330,7 @@ aws4@^1.8.0:
resolved "https://registry.npmjs.org/aws4/-/aws4-1.9.1.tgz#7e33d8f7d449b3f673cd72deb9abdc552dbe528e"
integrity sha512-wMHVg2EOHaMRxbzgFJ9gtjOOCrI80OHLG14rxi28XwOW8ux6IiEbRCGGGqCtdAIg4FQCbW20k9RsT4y3gJlFug==
axios@^0.19.0:
axios@^0.19.0, axios@^0.19.2:
version "0.19.2"
resolved "https://registry.npmjs.org/axios/-/axios-0.19.2.tgz#3ea36c5d8818d0d5f8a8a97a6d36b86cdc00cb27"
integrity sha512-fjgm5MvRHLhx+osE2xoekY70AhARk3a6hkN+3Io1jc00jtquGvxYlKlsFUhmUET0V5te6CcZI7lcv2Ym61mjHA==