refactor: change file uploads#8811
Merged
jniles merged 1 commit intoJul 3, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors server-side file handling to remove the tempy dependency and to shift uploads/reports toward OS-standard data locations (via env-paths) or a configurable BHIMA_DATA_DIR, addressing the long-standing issue of uploads being wiped during rebuilds.
Changes:
- Replaces
tempywith a newutil.tempFilePath()helper for generating temporary file paths. - Refactors upload storage and report output directories to use
env-paths/BHIMA_DATA_DIRinstead of paths within the installation. - Updates dependencies to add
env-pathsand removetempy.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| server/lib/util.js | Adds a temp file path helper and updates Node built-in imports. |
| server/lib/uploader.js | Refactors upload directory selection/creation and multer storage configuration. |
| server/lib/ReportManager.js | Refactors report directory selection and removes lodash usage. |
| server/controllers/stock/lots.js | Switches barcode temp-file creation from tempy to util.tempFilePath(). |
| package.json | Adds env-paths, removes tempy. |
| package-lock.json | Updates lockfile to reflect dependency changes. |
| .env.sample | Replaces UPLOAD_DIR/REPORT_DIR with BHIMA_DATA_DIR and adjusts DEBUG defaults. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+42
to
46
| function getUploadDirectory() { | ||
| const defaultPath = process.env.BHIMA_DATA_DIR ? osPaths.data : process.env.BHIMA_DATA_DIR; | ||
| const uploadPath = path.resolve(defaultPath, 'uploads'); | ||
| return uploadPath.endsWith(path.sep) ? uploadPath : uploadPath + path.sep; | ||
| } |
Comment on lines
+25
to
+29
| const path = require('node:path'); | ||
| const fs = require('node:fs/promises'); | ||
| const multer = require('multer'); | ||
| const fs = require('fs'); | ||
| const debug = require('debug')('app:uploader'); | ||
|
|
||
| const osPaths = require('env-paths').default('bhima'); |
Comment on lines
+51
to
+57
| async function ensureUploadDirectoryExists() { | ||
| const uploadPath = getUploadDirectory(); | ||
| debug(`Using ${uploadPath} as the upload directory.`); | ||
| await fs.mkdir(uploadPath, { recursive: true}); | ||
| await fs.access(uploadPath, fs.constants.R_OK | fs.constants.W_OK); | ||
| debug(`Successfully created ${uploadPath}.`); | ||
| } |
Comment on lines
68
to
79
| function Uploader(prefix, fields) { | ||
| // format the upload directory. Add a trailing slash for consistency | ||
| const hasTrailingSlash = (prefix[prefix.length - 1] === '/'); | ||
| const linkDirectory = path.join(dir, hasTrailingSlash ? prefix : `${prefix}/`); | ||
| const uploadDirectory = getUploadDirectory(); | ||
|
|
||
| // configure the storage space using multer's diskStorage. This will allow | ||
| const storage = multer.diskStorage({ | ||
| destination : async (req, file, cb) => { | ||
|
|
||
| try { | ||
| // NOTE: need absolute path here for mkdirp | ||
| const fullFolderPath = path.join(fsdir, hasTrailingSlash ? prefix : `${prefix}/`); | ||
| debug(`creating upload directory ${fullFolderPath}.`); | ||
| await mkdirp(fullFolderPath); | ||
| cb(null, fullFolderPath); | ||
| } catch (err) { | ||
| cb(err); | ||
| } | ||
| }, | ||
| destination : (req, file, cb) => { cb(null, uploadDirectory); }, | ||
| filename : (req, file, cb) => { | ||
| const id = uuid(); | ||
|
|
||
| // ensure that a link is passed to the req.file object | ||
| file.link = `${linkDirectory}${id}`; | ||
| const linkDirectory = path.resolve(uploadDirectory, prefix); | ||
| file.link = path.resolve(linkDirectory, id); | ||
| debug(`Storing file in ${file.link}.`); | ||
| cb(null, id); |
Comment on lines
+104
to
+109
| ensureUploadDirectoryExists(); | ||
|
|
||
|
|
||
| // attach the relative upload directory path for outside consumption | ||
| exports.directory = getUploadDirectory(); | ||
|
|
Comment on lines
19
to
+23
| const debug = require('debug')('ReportManager'); | ||
| const path = require('path'); | ||
| const tempy = require('tempy'); | ||
| const osPaths = require('env-paths').default('bhima'); | ||
| const path = require('node:path'); | ||
| const datauri = require('datauri'); | ||
| const fs = require('fs/promises'); | ||
| const fs = require('node:fs/promises'); |
Comment on lines
174
to
+176
| try { | ||
| await fs.access(metadata.enterprise.logopath, fs.constants.R_OK); | ||
| metadata.enterprise.logoDataURI = await datauri( | ||
| metadata.enterprise.logopath, | ||
| ); | ||
| metadata.enterprise.logoDataURI = await datauri(metadata.enterprise.logopath); |
Comment on lines
+328
to
+338
| function tempFilePath({ name, extension } = {}) { | ||
| const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'bhima-')); | ||
|
|
||
| if (name) { | ||
| return path.join(dir, name); | ||
| } | ||
|
|
||
| const randomName = crypto.randomBytes(16).toString('hex'); | ||
| const fileName = extension ? `${randomName}.${extension}` : randomName; | ||
| return path.join(dir, fileName); | ||
| } |
Comment on lines
593
to
594
| const zipped = await zipFiles(tmpCsvFile, tmpPdfFile); | ||
| res.download(zipped); |
Comment on lines
27
to
+33
| # define logging level for the npm debug module. | ||
| DEBUG=app,errors | ||
| DEBUG=app,http,errors | ||
|
|
||
| # define the directory where reports, files, and images will be uploaded. | ||
| # Upload directory requirements: | ||
| # - Must be writable by the application user. | ||
| # - Must be a relative directory within the Bhima installation. | ||
| UPLOAD_DIR='client/upload' | ||
|
|
||
| # Report directory (Define where reports will be saved on the server) | ||
| REPORT_DIR='' | ||
| # - be read/writable by the application user. | ||
| BHIMA_DATA_DIR='client/upload' |
jniles
force-pushed
the
6274-refactor-file-uploads
branch
2 times, most recently
from
a051ebd to
9bac70b
Compare
Removes tempy and replaces with nodejs's internal temporary files. It also uses os default paths for data and configuration where an upload path is not provided. Closes third-culture-software#6274.
jniles
force-pushed
the
6274-refactor-file-uploads
branch
from
9bac70b to
7bd953d
Compare
Merged
via the queue into
third-culture-software:master
with commit Jul 3, 2026
ca3121d
3 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes tempy and replaces with nodejs's internal temporary files. It also uses os default paths for data and configuration where an upload path is not provided.
One breaking change is that I unified the report directory and the upload directory so they both use the same base directory. This directory is found in
BHIMA_DATA_DIRas an environmental variable.Closes #6274.