Issue #88, #90, and #92 #94
Conversation
|
Worked on issue #90. Made the following changes: |
|
Worked on #92. Made the following changes: Created two feature modules: Updated HTML files to load a single module script each: |
|
Issue #88 : |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the codebase to consolidate fetch calls into a centralized service module (objectService.js) and extract utility functions into a dedicated utils module (generalUtils.js), addressing issues #88, #90, and #92. The changes improve maintainability, testability, and follow the DRY principle by eliminating duplicated code across multiple files.
Key changes:
- Created
web/js/services/objectService.jsto centralize all fetch operations (RERUM CRUD operations, manifest fetching, and HTML component fetching) - Created
web/js/utils/generalUtils.jsto house shared utilities (logger, broadcast, thumbnailGenerator) - Refactored
utilities.jsto import and re-export from the new modules instead of containing inline implementations - Introduced feature entry files (
tools-feature.js,sandbox-feature.js) to consolidate module imports for each HTML page
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/js/services/objectService.js | New service module centralizing all fetch operations including RERUM API calls, manifest loading, and HTML component fetching |
| web/js/utils/generalUtils.js | New utility module containing logger, broadcast, and thumbnailGenerator functions extracted from utilities.js |
| web/js/utilities.js | Refactored to import and re-export functions from the new service and utils modules, removing inline implementations |
| web/js/playground.js | Updated to use fetchFooter and fetchMenu from objectService instead of inline fetch calls |
| web/js/tools.js | Updated to use fetchManifest from objectService |
| web/js/sandbox.js | Added import of fetchFooter from objectService |
| web/js/features/tools-feature.js | New feature entry point consolidating imports for tools.html |
| web/js/features/sandbox-feature.js | New feature entry point consolidating imports for sandbox.html |
| web/js/json-utils.js | Added JSDoc comments for prettifyJSON and validateJSON functions, minor whitespace fix |
| web/js/toolsCatalog.js | Added JSDoc header comment describing the catalog structure |
| web/tools.html | Updated to load single feature entry script instead of multiple module scripts |
| web/sandbox.html | Updated to load single feature entry script instead of multiple module scripts |
| web/index.html | Removed unused index.js script tag |
| web/about.html | Removed unused about.js script tag |
Comments suppressed due to low confidence (1)
web/js/sandbox.js:1
- Unused import fetchFooter.
import { fetchFooter } from './services/objectService.js';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 500: "Internal server error", | ||
| 503: "Server down time", | ||
| } | ||
| logger.warn(errorMessages[response.status] ?? `Unhandled HTTP Error ${response.status}`) |
There was a problem hiding this comment.
The logger object is used but never imported. Add import { logger } from '../utils/generalUtils.js'; at the top of the file.
| import { fetchFooter } from './services/objectService.js'; | ||
|
|
There was a problem hiding this comment.
The fetchFooter function is imported but never used in this file. Remove this unused import or add the missing implementation that uses it.
| import { fetchFooter } from './services/objectService.js'; |
|
Issue #90 |
|
Issue #92 |
Worked on issue #88. Moved fetch calls rom playground.js, tools.js, and sandbox.js into web/js/services/objectService.js. Also moved fetch calls from utilities.js to objectService.js to promote easier maintenance, testability, and better adherence to the DRY principle.