Closed mblomdahl closed 1 month ago
In the file OwnTube.tv/lib/peertubeVideosApi.test.ts :
Mocking the API calls: Currently, the tests directly call the PeerTube API. It might be better to mock the API calls using a library like jest-mock or no-mock to isolate the tests and avoid making external requests during testing.
The tests currently focus on successful responses. Consider adding tests that handle error scenarios like network issues or invalid responses from the API.
Edge Cases: You could explore testing edge cases like fetching 0 videos or exceeding the total available videos by a large margin.
Refactoring for Readability: The test for video properties can be refactored into a separate function to improve readability.
in the file OwnTube.tv/lib/peertubeVideosApi.ts
Code Duplication: private createAxiosInstance(peertubeHost: string, debugLogging: boolean): void { const baseURL = https://${peertubeHost}/api/v1/; this.instance = axios.create({ baseURL, headers: { "Content-Type": "application/json", "User-Agent": "OwnTube.tv/1.0.0 (https://app.owntube.tv/)", }, });
// Request interceptor this.instance.interceptors.request.use((config) => { if (debugLogging) { console.debug(Requesting '${baseURL}${config.url}' with '${JSON.stringify(config.params)}'); } return config; });
// Response interceptor this.instance.interceptors.response.use( (response) => { if (debugLogging) { console.debug(Received response with status ${response.status}); } return response; }, (error) => { if (debugLogging) { console.error(Error response: ${JSON.stringify(error.response)}); } return Promise.reject(error); } ); }
instance.interceptors.request.use((config) => {
if (this.debugLogging) {
console.debug(Requesting '${config.baseURL}${config.url}' with '${JSON.stringify(config.params)}'
);
}
return config;
});
instance.interceptors.response.use((response) => { if (this.debugLogging) { console.debug( Received response with status ${response.status} and data '${JSON.stringify(response.data).substring(0, 255)}...', ); } return response; });
this.instance = instance; }
Using function parameters: You can pass parameters such as debugLogging as function arguments instead of using class properties. This makes the function more flexible. For example: private createAxiosInstance(peertubeHost: string, debugLogging: boolean): void { const instance = axios.create({ baseURL: https://${peertubeHost}/api/v1/, headers: { "Content-Type": "application/json", "User-Agent": "OwnTube.tv/1.0.0 (https://app.owntube.tv/)", }, });
instance.interceptors.request.use((config) => { if (debugLogging) { console.debug(Requesting '${config.baseURL}${config.url}' with '${JSON.stringify(config.params)}'); } return config; });
instance.interceptors.response.use((response) => { if (debugLogging) { console.debug( Received response with status ${response.status} and data '${JSON.stringify(response.data).substring(0, 255)}...', ); } return response; });
this.instance = instance; }
Extract Constants: You are using some values, such as this.maxChunkSize, multiple times. Extract them into constants to make the code more readable."
Error Handling: Instead of using try/catch inside the loop, you can move this logic outside the loop. This way, you'll avoid unnecessary error catching
@tryklick & @OGTor Rebased this on main
after merging #42 this morning. Should be good to merge!
@TOPDeveloperPB I have a hard time following your 2 comments here, it would be a lot easier if you use the GitHub review functionality + apply proper Markdown formatting to code samples.
Merged in #62
This service will be used to interact with the PeerTube backend API. It will be used to reliably retrieve videos when needed; at the moment, its functionality is limited to ...
In the future, this service will be extended to include more functionality, such as ...
Ping @OGTor & @ar9708 & @tryklick, FYI!
(Note: As an exercise, feel free to see if you can improve upon the
PeertubeVideosApi
service to make it more readable and understandable, and still passing all tests.)