Bug 33938

Summary:	Installer wants to upgrade even when the DB is up-to-date
Product:	Koha	Reporter:	Jonathan Druart <jonathan.druart>
Component:	Installation and upgrade (command-line installer)	Assignee:	Jonathan Druart <jonathan.druart>
Status:	Failed QA ---	QA Contact:	Testopia <testopia>
Severity:	normal
Priority:	P5 - low	CC:	dcook, m.de.rooy
Version:	unspecified
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	Trivial patch
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Attachments:	Bug 33938: Don't tell the DB is not up-to-date when it is Bug 33938: Don't tell the DB is not up-to-date when it is

Description Jonathan Druart 2023-06-07 11:44:37 UTC

If you hit /cgi-bin/koha/installer/install.pl and that the DB is up-to-date already, the screen shows "We are upgrading from Koha 23.06.00.002 to 23.06.00.002"

Comment 1 Jonathan Druart 2023-06-07 11:46:07 UTC Comment hidden (obsolete)

Created attachment 152098 [details] [review]
Bug 33938: Don't tell the DB is not up-to-date when it is

If you hit /cgi-bin/koha/installer/install.pl and that the DB is up-to-date already,
the screen shows "We are upgrading from Koha 23.06.00.002 to 23.06.00.002".

We should just say that everything is ok

Test plan:
Have an up-to-date DB, then hit /cgi-bin/koha/installer/install.pl
Notice that the message changed and is more correct with the patch
applied.

Comment 2 Katrin Fischer 2023-06-09 09:32:20 UTC

Created attachment 152232 [details] [review]
Bug 33938: Don't tell the DB is not up-to-date when it is

If you hit /cgi-bin/koha/installer/install.pl and that the DB is up-to-date already,
the screen shows "We are upgrading from Koha 23.06.00.002 to 23.06.00.002".

We should just say that everything is ok

Test plan:
Have an up-to-date DB, then hit /cgi-bin/koha/installer/install.pl
Notice that the message changed and is more correct with the patch
applied.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Comment 3 David Cook 2023-07-10 23:14:22 UTC

This feels wrong to me. We shouldn't have the comparison check in the templates. It should be in the controller script.

Also, it doesn't take into account atomic updates. 

I'd raised bug 34088 to handle this scenario for updatedatabase.pl but hadn't gotten around to install.pl.

Comment 4 Jonathan Druart 2023-07-11 06:59:36 UTC

(In reply to David Cook from comment #3)
> This feels wrong to me. We shouldn't have the comparison check in the
> templates. It should be in the controller script.

Are you suggesting to make the comparison in the pl then pass a variable 'needs_upgrade' if dbversion != kohaversion?

Comment 5 Marcel de Rooy 2023-07-11 11:42:36 UTC

If I hit this URL, I need to login first with the admin user to discover that the version is the same. We could do better than that?

Comment 6 David Cook 2023-07-11 23:26:48 UTC

(In reply to Jonathan Druart from comment #4)
> Are you suggesting to make the comparison in the pl then pass a variable
> 'needs_upgrade' if dbversion != kohaversion?

Well, there is already an "upgrading" variable which seems to be set without checking that upgrading is necessary. I think that we should fix it there.

Comment 7 David Cook 2023-07-11 23:34:00 UTC

(In reply to Marcel de Rooy from comment #5)
> If I hit this URL, I need to login first with the admin user to discover
> that the version is the same. We could do better than that?

That's a good point too. 

Near the top of install.pl, we could do an upgrade check (I'd suggest the one from bug 34088) and if an upgrade isn't required, redirect to mainpage.pl.