1571: Import stores backend #1606

f1sh1918 · 2024-08-28T14:26:12Z

Short description

As a user i want to import stores accepting stores via csv. The backend part is implemented here

Proposed changes

create reusable functions for mapping and storing the accepting store data in the database
add a dryRun function to check how many stores are affected, before applying db actions
add missing react component tests
add missing tests for backend functions
TODO integration test for the mutation (Add test for importCsvStores #1605), since i couldn't get mocking bearer token for and have to investigate more

Side effects

the normal store import function may be affected due to the refactoring. Please test if the standard store import (runConfig) still works. Here you can find a csv with the correct scheme.
https://github.com/digitalfabrik/sozialpass-nuernberg-data/blob/main/nuernberg-akzeptanzstellen.csv
Note: If you have added the koblenz project to your config.local.yml you have to set this
pipelineName: BerechtigungskarteShowcase for koblenz to skip the store import via pipeline

Testing

Create a Project Store Manager and login
Go to "Akzeptanzpartner verwalten"
Add a csv, data can be found here (1475: Csv store import module #1572)
Import the stores and check the database tables
Open native nuernberg app and check if the stores and the correct data is available
Do the same using "testlauf" flag with a different csv and check that the database was not affected

Resolved issues

Fixes: #1571

…ckend and frontend

…d components, add dryRun option

f1sh1918 · 2024-08-28T14:29:14Z

administration/src/bp-modules/cards/CreateCardsButtonBar.test.tsx

@@ -8,9 +8,6 @@ import bayernConfig from '../../project-configs/bayern/config'
 import CreateCardsButtonBar from './CreateCardsButtonBar'

 jest.useFakeTimers()
-jest.mock('csv-stringify/browser/esm/sync', () => ({
-  stringify: jest.fn(),
-}))


moved this to the jest.setup for all tests using it

administration/src/bp-modules/stores/StoresTable.tsx

...nd/src/main/kotlin/app/ehrenamtskarte/backend/stores/importer/nuernberg/steps/DownloadCsv.kt

...c/main/kotlin/app/ehrenamtskarte/backend/stores/webservice/AcceptingStoresMutationService.kt

administration/src/bp-modules/stores/StoresImportResult.tsx

administration/src/bp-modules/stores/StoresCSVInput.tsx

...c/main/kotlin/app/ehrenamtskarte/backend/stores/webservice/AcceptingStoresMutationService.kt

...rc/main/kotlin/app/ehrenamtskarte/backend/stores/database/repos/AcceptingStoresRepository.kt

seluianova · 2024-09-03T17:23:15Z

backend/src/main/kotlin/app/ehrenamtskarte/backend/stores/utils/StoreFieldCleaner.kt

+/** Returns null if string can't be trimmed f.e. empty string
+ * Removes subsequent whitespaces
+ * */
+fun String?.clean(removeSubsequentWhitespaces: Boolean = true): String? {


nit-picky:
if we call this function with removeSubsequentWhitespaces = false, we get the same effect as trim().
so I'm not quite sure why this extra branch of logic is needed?
when we need only trim() why not just call trim() ?

i adjusted this function to return null for empty string and added removeSubsequentStrings false if it is used for the csv import via administration

Was it on purpose to remove replaceNa()?

can you give me an explanation about what that notes should be that will be replaced with this function. i couldn't find any good example for that.

i readded replaceNa() just in case to not break anything.
It would be good to add an example in the tests @steffenkleinle. But this can be done is a separate task
I couldn't find much information about that and how a string with such a note looks like

seluianova · 2024-09-03T18:17:35Z

One more finding:
If some fields are null, they are inserted as empty in the database.
For example, the first 2 records (with empty emails) were inserted from CSV and the rest were inserted the old way:

I assume null-values are expected in such cases.

seluianova · 2024-09-03T19:46:09Z

backend/src/main/kotlin/app/ehrenamtskarte/backend/stores/utils/MapCsvToStore.kt

+
+fun mapCsvToStore(csvStore: CSVAcceptingStore): AcceptingStore {
+    return AcceptingStore(
+        csvStore.name.clean()!!, COUNTRY_CODE, csvStore.location.clean()!!, csvStore.postalCode.clean()!!, csvStore.street.clean()!!, csvStore.houseNumber.clean()!!, "", csvStore.longitude!!.toDouble(), csvStore.latitude!!.toDouble(), csvStore.categoryId!!.toInt(), csvStore.email.clean()!!, csvStore.telephone.clean()!!, csvStore.homepage.clean()!!, csvStore.discountDE.orEmpty() + "\n\n" + csvStore.discountEN.orEmpty(), null, null


if discountDE and discountEN are null, I think the expected description should be null, or?
currently it's "\n\n"

yes i added a check

now if discountDE is null and discountEN is not null ("test description", for example), it's saved like "\n\ntest description".
probably it should be "test description"

addressed this in last commit by using a mutableList now its even better extendable if we want to add another language for discount someday

...c/main/kotlin/app/ehrenamtskarte/backend/stores/webservice/schema/types/CSVAcceptingStore.kt

ztefanie

Tested normal store import via run config. Works as expected

Is it expected that project admins cannot create users with the role store import? Because only we want to use this feature? If not, users need the option to select the PROJECT_STORE_MANAGER role, when creating a new user.

Add a new file to the other csv resources as an example of the csv import please.

View is disabled for project with disabled upload and visible for nuernberg as expected.

import worked as expected.

administration/src/bp-modules/stores/StoresImportAlert.tsx

ztefanie · 2024-09-09T11:33:47Z

administration/src/bp-modules/stores/StoresImportController.tsx

@@ -29,6 +31,13 @@ const StoresImportController = (): ReactElement => {
  return <StoresImport fields={storeManagement.fields} />
 }

+const CenteredSpinner = styled(Spinner)`
+  z-index: 999;


suggest to use 500 so we could possible put something before in the future.

but what should be before an loading spinner f.e ? even its shown over a modal it should be highest index

...c/main/kotlin/app/ehrenamtskarte/backend/stores/webservice/AcceptingStoresMutationService.kt

f1sh1918 · 2024-09-09T13:00:07Z

Tested normal store import via run config. Works as expected

Is it expected that project admins cannot create users with the role store import? Because only we want to use this feature? If not, users need the option to select the PROJECT_STORE_MANAGER role, when creating a new user.

Add a new file to the other csv resources as an example of the csv import please.

View is disabled for project with disabled upload and visible for nuernberg as expected.

import worked as expected.

yes for the moment the store import is an internal function and not provided to customers. When we decide to sell that function, we have to add the possibility to create users with this role

seluianova

Maximale Dateigröße: 2MB

If I try to upload 2Mb file (it's appx 5500 lines), I'm getting an error on the backend side:

[JettyServerThreadPool-33] WARN io.javalin.Javalin - Body greater than max size (1000000 bytes)
io.javalin.http.HttpResponseException: Content Too Large
	at io.javalin.http.servlet.MaxRequestSize.throwContentTooLargeIfContentTooLarge(JavalinServletContext.kt:236)
	at io.javalin.http.Context.bodyAsBytes(Context.kt:147)
	at io.javalin.http.servlet.JavalinServletContext.access$bodyAsBytes$s-1678783089(JavalinServletContext.kt:65)
	at io.javalin.http.servlet.JavalinServletContext$body$2.invoke(JavalinServletContext.kt:133)
	at io.javalin.http.servlet.JavalinServletContext$body$2.invoke(JavalinServletContext.kt:133)
	at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
	at io.javalin.http.servlet.JavalinServletContext.getBody(JavalinServletContext.kt:133)
	at io.javalin.http.servlet.JavalinServletContext.bodyAsBytes(JavalinServletContext.kt:134)
	at io.javalin.http.Context.body(Context.kt:138)
	at io.javalin.plugin.bundled.DevLoggingPlugin.httpDevLogger(DevLoggerPlugin.kt:55)
	at io.javalin.plugin.bundled.DevLoggingPlugin.onInitialize$lambda$0(DevLoggerPlugin.kt:29)
	at io.javalin.http.servlet.JavalinServlet.writeResponseAndLog(JavalinServlet.kt:114)
	at io.javalin.http.servlet.JavalinServlet.handleSync(JavalinServlet.kt:68)
	at io.javalin.http.servlet.JavalinServlet.handle(JavalinServlet.kt:50)
	at io.javalin.http.servlet.JavalinServlet.service(JavalinServlet.kt:30)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:587)
	at io.javalin.jetty.JavalinJettyServlet.service(JavalinJettyServlet.kt:52)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:587)
	at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:764)
	at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:529)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:221)
	at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1580)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:221)
	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1381)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:176)
	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:484)
	at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1553)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:174)
	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1303)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:129)
	at org.eclipse.jetty.server.handler.StatisticsHandler.handle(StatisticsHandler.java:173)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122)
	at org.eclipse.jetty.server.Server.handle(Server.java:563)
	at org.eclipse.jetty.server.HttpChannel$RequestDispatchable.dispatch(HttpChannel.java:1598)
	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:753)
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:501)
	at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:287)
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:314)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
	at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.runTask(AdaptiveExecutionStrategy.java:421)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:390)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:277)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.run(AdaptiveExecutionStrategy.java:199)
	at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:411)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:969)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1194)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1149)
	at java.base/java.lang.Thread.run(Thread.java:840)

f1sh1918 · 2024-09-13T18:44:50Z

Maximale Dateigröße: 2MB

If I try to upload 2Mb file (it's appx 5500 lines), I'm getting an error on the backend side:

[JettyServerThreadPool-33] WARN io.javalin.Javalin - Body greater than max size (1000000 bytes)
io.javalin.http.HttpResponseException: Content Too Large
	at io.javalin.http.servlet.MaxRequestSize.throwContentTooLargeIfContentTooLarge(JavalinServletContext.kt:236)
	at io.javalin.http.Context.bodyAsBytes(Context.kt:147)
	at io.javalin.http.servlet.JavalinServletContext.access$bodyAsBytes$s-1678783089(JavalinServletContext.kt:65)
	at io.javalin.http.servlet.JavalinServletContext$body$2.invoke(JavalinServletContext.kt:133)
	at io.javalin.http.servlet.JavalinServletContext$body$2.invoke(JavalinServletContext.kt:133)
	at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
	at io.javalin.http.servlet.JavalinServletContext.getBody(JavalinServletContext.kt:133)
	at io.javalin.http.servlet.JavalinServletContext.bodyAsBytes(JavalinServletContext.kt:134)
	at io.javalin.http.Context.body(Context.kt:138)
	at io.javalin.plugin.bundled.DevLoggingPlugin.httpDevLogger(DevLoggerPlugin.kt:55)
	at io.javalin.plugin.bundled.DevLoggingPlugin.onInitialize$lambda$0(DevLoggerPlugin.kt:29)
	at io.javalin.http.servlet.JavalinServlet.writeResponseAndLog(JavalinServlet.kt:114)
	at io.javalin.http.servlet.JavalinServlet.handleSync(JavalinServlet.kt:68)
	at io.javalin.http.servlet.JavalinServlet.handle(JavalinServlet.kt:50)
	at io.javalin.http.servlet.JavalinServlet.service(JavalinServlet.kt:30)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:587)
	at io.javalin.jetty.JavalinJettyServlet.service(JavalinJettyServlet.kt:52)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:587)
	at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:764)
	at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:529)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:221)
	at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1580)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:221)
	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1381)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:176)
	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:484)
	at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1553)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:174)
	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1303)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:129)
	at org.eclipse.jetty.server.handler.StatisticsHandler.handle(StatisticsHandler.java:173)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122)
	at org.eclipse.jetty.server.Server.handle(Server.java:563)
	at org.eclipse.jetty.server.HttpChannel$RequestDispatchable.dispatch(HttpChannel.java:1598)
	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:753)
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:501)
	at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:287)
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:314)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
	at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.runTask(AdaptiveExecutionStrategy.java:421)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:390)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:277)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.run(AdaptiveExecutionStrategy.java:199)
	at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:411)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:969)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1194)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1149)
	at java.base/java.lang.Thread.run(Thread.java:840)

@seluianova
OK thx for testing.
i realized that a file with 2MB has even a larger content size (HTTP Request)
Example:

To ensure that nothing breaks i set the value to 5MB in the backend. But i have no clue how to calculate this on the basis of a 2MB csv file :/
But now the upload for large files work.
It even should now be sufficient for the user import i guess. But we may ask in the meeting with koblenz and create example files

administration/src/bp-modules/stores/StoresImportDuplicates.tsx

f1sh1918 · 2024-09-14T08:24:14Z

Uploading 10000 stores take 80seconds. I added a aprox. duration to the alert dialog, for mass uploads to inform the user that it may take a while

steffenkleinle

Tested on firefox, did not find any issues! Great work 🎉

Is it on purpose that this does not disappear again on its own?

administration/src/bp-modules/stores/StoresCSVInput.tsx

administration/src/bp-modules/stores/StoresImportAlert.tsx

steffenkleinle · 2024-09-17T12:18:24Z

backend/src/main/kotlin/app/ehrenamtskarte/backend/stores/utils/StoreFieldCleaner.kt

+/** Returns null if string can't be trimmed f.e. empty string
+ * Removes subsequent whitespaces
+ * */
+fun String?.clean(removeSubsequentWhitespaces: Boolean = true): String? {


Was it on purpose to remove replaceNa()?

...c/main/kotlin/app/ehrenamtskarte/backend/stores/webservice/AcceptingStoresMutationService.kt

...rc/main/kotlin/app/ehrenamtskarte/backend/stores/database/repos/AcceptingStoresRepository.kt

f1sh1918 · 2024-09-17T13:41:14Z

Tested on firefox, did not find any issues! Great work 🎉

Is it on purpose that this does not disappear again on its own?

Yes

seluianova · 2024-09-17T18:52:45Z

I would do a final review once you have addressed Steffen's comments if you don't mind, in order to re-test all together, but I think it should be good to go.

...ain/kotlin/app/ehrenamtskarte/backend/exception/webservice/exceptions/DatabaseIOException.kt

seluianova · 2024-09-23T13:32:01Z

backend/src/main/kotlin/app/ehrenamtskarte/backend/stores/utils/MapCsvToStore.kt

+    discounts.removeIf { it.isNullOrEmpty() }
+    return discounts.joinToString("\n\n")
+}
+fun mapCsvToStore(csvStore: CSVAcceptingStore): AcceptingStore {


optional:
do we want to handle an error, when csvStore failed to be mapped into AcceptingStore?
it can happen for example if categoryId can't be parsed to Integer, or coordinates cannot be parsed to Double.
(or if some fields are missed)
currently the service returns 500 internal server error in such case.

you can add an improvement github issue. I think this pr is now big enough :)

f1sh1918 · 2024-09-23T13:42:14Z

I would do a final review once you have addressed Steffen's comments if you don't mind, in order to re-test all together, but I think it should be good to go.

i think i addressed all import ones @seluianova

seluianova · 2024-09-23T14:02:56Z

nit-picky thought:
currently we have a field configuration in administration, where the list of fields can be customized.
but, in the backend the list of fields is fixed anyway.
for example, if I remove discountEN from the configuration, I still can't upload stores without that field because the import into the backend will fail.

and vice versa:
in the frontend we have set some fields as mandatory, but in the backend they are not mandatory.
this means I can send an importAcceptingStores request directly to the backend (since this is technically possible) and upload stores with null-name, for example.

I'm not pushing to change this behavior right now, just highlighting it for discussion

I think at best this behavior between the front and back should be unified

f1sh1918 · 2024-09-23T14:40:15Z

nit-picky thought: currently we have a field configuration in administration, where the list of fields can be customized. but, in the backend the list of fields is fixed anyway. for example, if I remove discountEN from the configuration, I still can't upload stores without that field because the import into the backend will fail.

and vice versa: in the frontend we have set some fields as mandatory, but in the backend they are not mandatory. this means I can send an importAcceptingStores request directly to the backend (since this is technically possible) and upload stores with null-name, for example.

I'm not pushing to change this behavior right now, just highlighting it for discussion

I think at best this behavior between the front and back should be unified

Absolutely valid input. I think this should be added in the cleanup.
I think the current issue is that we have two ways (1. the old via csv file and backend cron import job, that will be deleted, 2. Upload via gui)
If we ran the first accepting store import on production we can delete the old import and improve CSVAcceptingStore data class
#1591

Please feel free to add also other improvements or cleanups to that issue

seluianova

👍
thanks for the great effort!

I think this should be added in the cleanup

not sure if validation on the backend is related to the clean up though, but maybe I'm missing something.
I think it could just be added to the mutation and it wouldn't affect the old import.
I see it like a separate issue - add the same validation in the backend as on the frontend.
I can create a ticket for that if it sounds reasonable to you.

f1sh1918 · 2024-09-23T16:46:00Z

👍 thanks for the great effort!

I think this should be added in the cleanup

not sure if validation on the backend is related to the clean up though, but maybe I'm missing something. I think it could just be added to the mutation and it wouldn't affect the old import. I see it like a separate issue - add the same validation in the backend as on the frontend. I can create a ticket for that if it sounds reasonable to you.

ah okay now i got it. Yes then please create a separate task

f1sh1918 added 3 commits August 26, 2024 18:17

1571: add mapping, store data in tables

90415fb

1571: refactoring cleanup and store functions, add result model to ba…

3de4630

…ckend and frontend

1571: add error handling, add tests for backend functions and fronten…

12deb10

…d components, add dryRun option

f1sh1918 requested review from steffenkleinle, ztefanie and seluianova as code owners August 28, 2024 14:26

f1sh1918 commented Aug 28, 2024

View reviewed changes

administration/src/bp-modules/stores/StoresTable.tsx Show resolved Hide resolved

f1sh1918 force-pushed the 1571-import-stores-backend branch from 31424f6 to 05e9a3e Compare August 28, 2024 14:35

1571: fix table height issue

cf835cb

f1sh1918 force-pushed the 1571-import-stores-backend branch from 05e9a3e to cf835cb Compare August 28, 2024 14:49

1571: add proper loading spinner

29e2548