Enable to import vac as CSV #993
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
|
AliKdhim87
force-pushed
the
enable-to-import-vac-as-csv
branch
from
3a20f65 to
c06cbd7
Compare
| return DOMPurify(window); | ||
| }); | ||
| const domPurify = createDOMPurify(); | ||
| export const sanitizeHTML = memoize((html: string) => domPurify.sanitize(html, { FORBID_ATTR: ['style'] })); |
There was a problem hiding this comment.
Ik zou geen memoize gebruiken hier, om memory usage te verminderen.
| @@ -46,7 +48,12 @@ const corsOption: CorsOptions = { | |||
| }; | |||
| const apiSpec = path.join(__dirname, './docs/openapi.yaml'); | |||
| const app = express(); | |||
| // Multer file upload middleware. | |||
| // The order is important, so this should be before the express.json() middleware to parse the file. | |||
| const upload = multer({ dest: 'tmp/uploads/' }); | |||
There was a problem hiding this comment.
Deze temporary file moet ook weer verwijderd worden.
There was a problem hiding this comment.
Het is opgelost door gebruik te maken van await fs.promises.unlink(filePath) in plaats van fs.unlinkSync(filePath).
| .on('end', () => { | ||
| if (hasRequiredColumns) { | ||
| const sanitizedResults = results.map((result) => { | ||
| const DOMPurifyHTML = sanitizeHTML(result?.antwoord); |
There was a problem hiding this comment.
Je gebruikt nu "constructor case" voor een variable. Ik stel voor om domPurifyHTML te gebruiken, om duidelijk te maken dat het geen constructor is maar een waarde.
| let hasRequiredColumns = true; | ||
|
|
||
| // Create read stream for CSV file | ||
| return new Promise<any[]>((resolve, reject) => { |
There was a problem hiding this comment.
Kunnen we any aanpassen naar een type?
| (fs.createReadStream as jest.Mock).mockReturnValue(mockStream); | ||
|
|
||
| const result = await processCsvFile(filePath, ['vraag', 'antwoord']); | ||
| expect(result).toEqual([ |
There was a problem hiding this comment.
Ik zou hier ook even de type controleren, met bijv: const expectedResults: SomeType[] = ...
| try { | ||
| // Process the CSV file and sanitize results | ||
| const isAuthHasToken = req.headers?.authorization?.startsWith('Token'); | ||
| const tokenAuth = isAuthHasToken ? req.headers?.authorization?.split(' ')[1] : req.headers?.authorization; |
There was a problem hiding this comment.
Als je dan toch al split doet, zou ik startsWith('Token') vervangen door arr[0].
Je kunt iets meer flexibel zijn met split(/\s+/)
AliKdhim87
force-pushed
the
enable-to-import-vac-as-csv
branch
from
c06cbd7 to
434a366
Compare
AliKdhim87
force-pushed
the
enable-to-import-vac-as-csv
branch
from
5959439 to
cd0dfb4
Compare
AliKdhim87
force-pushed
the
enable-to-import-vac-as-csv
branch
from
cd0dfb4 to
033875f
Compare
AliKdhim87
force-pushed
the
enable-to-import-vac-as-csv
branch
from
033875f to
09a22e7
Compare
AliKdhim87
force-pushed
the
enable-to-import-vac-as-csv
branch
from
09a22e7 to
b51909e
Compare
Strapi v4 does not natively support bulk creation endpoints, which can make handling large numbers of entries inefficient. While a custom controller could be implemented to handle multiple entries in a single request, this would require backend adjustments and might not be worth the effort, as this feature is only needed once. Additionally, bulk creation support may be available in Strapi v5, making this solution more temporary.
To address this, I implemented a loop that processes each entry individually while controlling concurrency with p-limit. This ensures no more than 5 requests are sent at once, preventing server overload and improving performance. By using p-limit, the system remains stable and responsive, even when processing multiple entries concurrently.
issue