Development#378
Conversation
Changed Collections Parameter for Save()
* getting started with Vault * newer Vault * tests * removing redundant code * proper tests passing * sample Vault * Bryan's refusal to .jsonld makes this not work as expected * loading resources This is incomplete by design. We need to ask Vault to add any resources we want resolved * expanding the Manifest a bit * touch up for merge * touch up for merge --------- Co-authored-by: Bryan Haberberger <bryan.j.haberberger@slu.edu>
* Current Project IIIF manifest Creation * Refactored the exportManifest() and endpoint name * Changed the exportManifest() function * TPEN ID error handled * endpoint to move the manifest.json to TPEN-Static-Dev * Changed @id to id * Getting all Ids * Console Clean up * Added env variables * Adding Imports * Adding Imports * Deleting Ids * Updating Logic Added * User not a member cannot change the manifest * Removing project2 * Adding comments to functions
* add hotkeys service * hotkey endpoints * aggregate hotkeys during project retrieval * specify hotkey fields to include * cleanup * Update Hotkeys.js * Update ProjectFactory.mjs * Return hotkeys as an Array of Strings * aligning with Class changes * remove create, since .save is not acting correctly * cleanup and drop .post * tests restored no Jest here, just checking exists. * tests and sinon upgrade * no db tests directly * Update exists_unit.test.mjs * putting post back in... * adding create back with safety * adding upsert to accomodate bad errors * Update Hotkeys.js * uncatch to let errors through * expect the errors to come back * switch to jest tests --------- Co-authored-by: cubap <cubap@slu.edu>
* Adding endpoint to save changes from the layer * Adding New Layer * No Empty Label and no Annotations * Updated new Layer * Adding Items to partOf * Changing id convention * Removing updating layer * Annotation Change * Adding Delete Endpoint * not sure... This type of thing? * Label Change * Added Layer Class * Adding rerum ids to delete and add * Changing tests * Update exists.test.mjs * Changing LayerLabel * Changes AnnotationCollection Structure * Update Pages API * Updating partOf Ids in case of any change * Adding Layer Metadata Label Change * example results as a base for comments, delete these files before merge. * Adding AddLayer Commenting Rest of the APIs * Added DeleteLayer Back * Deleting json files * Updates to the comments * Making Project to Layer Changes * Renaming layerAnnotationCollection * Removing UpdateOne() changes * Removing UpdateOne() changes * Removing UpdateOne() changes * Error Handled for no ProjectID or LayerID * Clear Code * send() instead of json() --------- Co-authored-by: cubap <cubap@slu.edu> Co-authored-by: Bryan Haberberger <bryan.j.haberberger@slu.edu>
GitHub per @mepripri
…pleenv-file 91 create sampleenv file
* determine data type by content * Removing type dependencies - Took controller and type out of controller - Added a function to detect the data types and assign the correct collection * matching tests to code move
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a comprehensive environment configuration refactoring and documentation improvements for TPEN Services. The main goal is to separate safe defaults from secrets, improve developer onboarding, and add better tooling support for AI assistants.
Key Changes:
- Introduces layered environment configuration with
config.env,.env.development, and.env.productiontemplates - Adds
env-loader.jsfor consistent environment variable loading across all entry points - Removes the
Hotkeysclass and migrates functionality to projectinterfacesnamespace - Updates default port from 3001 to 3011
- Adds smoke tests for post-deployment validation
- Improves documentation (README.md, CONTRIBUTING.md, new CONFIG.md)
Reviewed Changes
Copilot reviewed 32 out of 38 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| env-loader.js | New module for layered environment variable loading with dotenv |
| config.env | Safe default configuration that can be committed to repository |
| .env.development | Development environment template with placeholder secrets |
| .env.production | Production environment template for deployment guidance |
| sample.env | Removed - replaced by layered configuration approach |
| bin/tpen3_services.js | Imports env-loader.js and updates default port to 3011 |
| app.js | Removes dotenv/dotenv-expand imports, adds CORS configuration |
| package.json | Moves @jest/globals to devDependencies, removes dotenv-expand, updates test scripts |
| classes/HotKeys/ | Removed - functionality migrated to project interfaces |
| classes/Project/ProjectFactory.js | Updates hotkeys to use interfaces namespace, improves image import logic |
| project/customMetadataRouter.js | New router for namespaced custom metadata with POST/PUT/GET endpoints |
| project/projectReadRouter.js | Adds interface filtering based on origin/query parameters |
| project/projectCreateRouter.js | Updates import-image endpoint to accept multiple image URLs |
| tests/smoke.test.js | New smoke tests for post-deployment validation |
| tests/mount.test.js | New route existence tests using Express routing table inspection |
| README.md | Comprehensive rewrite with quick start guide and configuration documentation |
| CONTRIBUTING.md | Updated with new environment configuration instructions |
| CONFIG.md | New comprehensive configuration guide |
| .github/workflows/ | Updated workflows to use new environment configuration |
| ecosystem.config.json | New PM2 configuration for cluster deployment |
| const { imageUrls, projectLabel } = req.body | ||
| if (!imageUrls || !projectLabel) { | ||
| return respondWithError(res, 400, "Image URL/URLs and project label are required") | ||
| } | ||
| if (!Array.isArray(imageUrls) || imageUrls.length === 0) { | ||
| return respondWithError(res, 400, "Image URLs must be a non-empty array") | ||
| } |
There was a problem hiding this comment.
The API change from accepting imageUrl (singular) to imageUrls (array) is a breaking change. Consider supporting both parameter names for backward compatibility, or ensure this breaking change is documented and coordinated with API consumers.
| { id: `${process.env.TPENSTATIC}/${_id}/contentPage.json`, type: "AnnotationPage", items: [ { id: `${process.env.TPENSTATIC}/${_id}/content.json`, type: "Annotation", motivation: "painting", body: { id: imageURL, type: "Image", format: mime.lookup(imageURL) || "image/jpeg", width: dimensions?.width, height: dimensions?.height, ...(isIIFImage && { service: [{ id: IIIFServiceURL, type: IIIFServiceJson?.type, profile: IIIFServiceJson?.profile }] }) }, target: `${process.env.TPENSTATIC}/${_id}/canvas-${index + 1}.json` } ] } | ||
| ], |
There was a problem hiding this comment.
This line is extremely long (over 450 characters) and deeply nested, making it very difficult to read and maintain. Consider breaking this into multiple lines with proper indentation or extracting the nested objects into separate variables.
| { id: `${process.env.TPENSTATIC}/${_id}/contentPage.json`, type: "AnnotationPage", items: [ { id: `${process.env.TPENSTATIC}/${_id}/content.json`, type: "Annotation", motivation: "painting", body: { id: imageURL, type: "Image", format: mime.lookup(imageURL) || "image/jpeg", width: dimensions?.width, height: dimensions?.height, ...(isIIFImage && { service: [{ id: IIIFServiceURL, type: IIIFServiceJson?.type, profile: IIIFServiceJson?.profile }] }) }, target: `${process.env.TPENSTATIC}/${_id}/canvas-${index + 1}.json` } ] } | |
| ], | |
| (() => { | |
| // Build the body object for the Annotation | |
| const body = { | |
| id: imageURL, | |
| type: "Image", | |
| format: mime.lookup(imageURL) || "image/jpeg", | |
| width: dimensions?.width, | |
| height: dimensions?.height, | |
| }; | |
| if (isIIFImage) { | |
| body.service = [ | |
| { | |
| id: IIIFServiceURL, | |
| type: IIIFServiceJson?.type, | |
| profile: IIIFServiceJson?.profile, | |
| } | |
| ]; | |
| } | |
| return { | |
| id: `${process.env.TPENSTATIC}/${_id}/contentPage.json`, | |
| type: "AnnotationPage", | |
| items: [ | |
| { | |
| id: `${process.env.TPENSTATIC}/${_id}/content.json`, | |
| type: "Annotation", | |
| motivation: "painting", | |
| body, | |
| target: `${process.env.TPENSTATIC}/${_id}/canvas-${index + 1}.json` | |
| } | |
| ] | |
| }; | |
| })() |
| if (isSuspiciousJSON(payload[key], Object.keys(payload[key])) || isSuspiciousValueString(payload[key], true)) { | ||
| payload[key] = { $$unsafe: payload[key] } | ||
| } |
There was a problem hiding this comment.
Potential type error: Object.keys(payload[key]) will throw if payload[key] is not an object. The code should check if payload[key] is an object before calling Object.keys() on it.
| "userClassTests": "node --experimental-vm-modules node_modules/jest/bin/jest.js -t user_class ", | ||
| "importTests": "node --experimental-vm-modules node_modules/jest/bin/jest.js -t importTests ", | ||
| "inviteMemberTests": "node --experimental-vm-modules node_modules/jest/bin/jest.js -t inviteMemberTests " | ||
| "allTests": "node --import ./env-loader.js --experimental-vm-modules node_modules/jest/bin/jest.js", |
There was a problem hiding this comment.
All test scripts now use --import ./env-loader.js. This is good for consistency, but ensure that test-specific environment files (.env.test) are properly handled by the env-loader, especially since NODE_ENV may not be set to 'test' in all test invocation scenarios.
| */ | ||
|
|
||
| import { jest } from "@jest/globals" | ||
| import request from "supertest" |
There was a problem hiding this comment.
Unused import request.
|
|
||
| import { jest } from "@jest/globals" | ||
| import request from "supertest" | ||
| import app from "../../app.js" |
There was a problem hiding this comment.
Unused import app.
| import Project from "../classes/Project/Project.js" | ||
| import dbDriver from "../database/driver.js" | ||
| import { ACTIONS, ENTITIES, SCOPES } from "./groups/permissions_parameters.js" | ||
| import screenContentMiddleware, { isSuspiciousJSON, isSuspiciousValueString } from "../utilities/checkIfSuspicious.js" |
There was a problem hiding this comment.
Unused import screenContentMiddleware.
| if (!validateID(id)) return respondWithError(res, 400, "The TPEN3 project ID provided is invalid") | ||
|
|
||
| try { | ||
| const project = new Project(id) |
There was a problem hiding this comment.
Unused variable project.
| } | ||
|
|
||
| // If no namespaces specified, remove all interfaces | ||
| if (!namespaces || namespaces.length === 0) { |
There was a problem hiding this comment.
This negation always evaluates to false.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This pull request introduces several improvements to developer and AI assistant documentation, project environment configuration, and local tooling for the TPEN Services repository. The main changes include adding detailed setup and workflow instructions for both developers and AI assistants, updating environment configuration templates for development and production, and enhancing the local Claude AI integration with custom status line and settings. These updates aim to make onboarding, development, and validation more robust and user-friendly.
Documentation and Developer Guidance Enhancements:
.claude/CLAUDE.mdfile with detailed instructions for AI assistants, including project overview, setup, testing, validation, environment requirements, workflow, debugging, CI/CD, and developer preferences..github/copilot-instructions.mdto clarify environment setup, configuration layering, port usage, and validation steps. Now recommends copying.env.developmentfor local development and documents the new configuration file structure. [1] [2]Environment Configuration Updates:
.env.developmentand.env.productiontemplates with clear separation of development and production settings, secrets, and documentation for each environment. These files guide users on how to configure their local or production environments securely and effectively. [1] [2]Claude AI Integration Improvements:
.claude/settings.jsonto configure Claude's environment, including status line customization, output token limits, and assistant preferences for code handling..claude/statusline-command.sh, a script that generates a color-coded status line in the terminal, displaying branch, model, cost, API time, and code changes for enhanced developer feedback when using Claude.