We had a race condition that could first display the report and then
replace it again with the "loading" spinner. That doesn't seem to happen
now that we changed the order of cable events.
I'm hiding a real bug here. There's a race condition when the cable event of
the finished report is sent before the loading spinner rendered. The
spinner then overwrites the report again. I added a spec for that but
don't have a solution yet.
I also noticed that the loading spinner is not displayed in testing but
we can assert on the CSS class of the container.
It was bad UX to show a link that doesn't work straight away. At the
time, it was the only way to download the report but now we send an
email which is a much better way to go.
I leave the rest of the code because we want to implement a reflex which
shows the download link once it's ready.
I added a system spec to verify that the download link can be generated
within the mailer in a background job. ActiveStorage is a bit particular
when it comes to genererating URLs and depending on the situation it may
generate a redirect URL, a proxy URL or link directly to the storage.
But we want a redirect URL here.
The `login_as_admin_and_visit` helper was used a lot but isn't really
shorter than:
login_as_admin
visit path_visit
Calling those methods separately reduces line length. It also removes
the potential impression that it may be more efficient to use the
helper. Now we have less indirection if one of the calls fails and see
the failing spec line straight away.
It was often used with a `visit` statement after which resulted in
unnecessary page loads. There was only one case where a `visit` was
expected but it wasn't needed either.
Forking worked in theory but crashed the browser in system specs. It
also came with many other hurdles and isn't well known solution in the
Rails community. Sidekiq can give us better control over execution
limits as well.
Each report can define formats for each column. But currency formatting
was also applied to all columns that had "price" in the name. Removing
this automation gives us more control and we can decide for each case.
At the moment, the currency formatting in Excel spreadsheets is not
ideal and it's easier to keep it as number.
This PR introduces a visual regression as prices are not formatted as
nicely but the columns can be used in calculations.
