Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DBManager: ORDER BY parameter added to getRecord(s) #770

Merged

Conversation

SJunkies
Copy link
Contributor

Summary

DBManager: ORDER BY parameter added to getRecord/getRecords.
All instances are now sorted in ascending order using the instance field / ID.
The web interface gets an incorrect instance order, which causes further problems.

Steps to reproduce:

  • create more than 1 instance, in my case, the new instance has become ID "1"
  • create 1 or 2 more instances (with different names) and you will have 3-4 instances with IDs : 0,1,2,3
  • delete instance with ID "1" and the remaining IDs are: 0,2,3
  • create a new instance again (in my case, i used the same name from previouse deleted instance again) and the IDs are now saved in DB order: 0,2,3,1
  • start and switch to your last instance (with ID "1", sorted and displayed as 4. instance).
  • If everything went to reproduce, the wrong instance name should be displayed ...

Why this is a Problem now?
The Web-Interface receive all instances in created order from DB, but uses the array indexes as instance IDs: 0 = 0, 1 = 2, 2 = 3, 3 = 1 ( content_index.js L#227/228. )
So every switch to another instance has the wrong instance settings...
and i'm not sure, but changed settings may where also saved for the wrong instance-ID!?

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of web configuration, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing setups:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's body (e.g. Fixes: #xxx[,#xxx], where "xxx" is the issue number)

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated (docs/docs/en)
  • Related tests have been updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

-> All instances are now sorted in ascending order using the instance ID
-> The web interface gets an incorrect instance order, which causes further problems
@hyperion-project
Copy link

Hello @SJunkies 👋

I'm your friendly neighborhood bot and would like to say thank you for
submitting a pull request to Hyperion!

So that you and other users can test your changes more quickly,
you can find your workflow artifacts here.

If you make changes to your PR, i create a new link to your workflow artifacts.

Best regards,
Hyperion-Project

@Paulchen-Panther
Copy link
Member

Paulchen-Panther commented Apr 17, 2020

Nachdem ich ein wenig probiert habe, funktioniert dann irgendwann gar nichts mehr.
Screens:

Screenshot

Screenshot from 2020-04-17 17-12-12

P.S.: @b1rdhous3 Da scheint was mit der Session nicht zu stimmen. Auch nach Löschung des Ordners .hyperion und neu aufrufen des WebUI bekomme ich diesen Fehler.

P.P.S.:

  1. wenn man eine laufende Instanz löscht schmiert Hyperion ab
  2. Hyperion erneut starten und schon kommt der Fehler.

@SJunkies
Copy link
Contributor Author

SJunkies commented Apr 17, 2020

@Paulchen-Panther
danke fürs Testen!
So ausgiebig hatte ich das garnicht erst gemacht ;)
Ich bin nur durch Zufall darauf gekommen, weil ich die Schritte wie oben beschrieben gemacht habe und beim Wechseln der Instanz gemerkt habe, das ich mich in einer eigentlich nicht aktiven/laufenden Instanz befinde, zumindest vom angezeigten Namen und der Config her.
Dem bin ich dann nur etwas weiter nachgegangen ...

@Paulchen-Panther
Copy link
Member

@SJunkies
Kann man deinen PR zusammenführen?

@SJunkies
Copy link
Contributor Author

@SJunkies
Kann man deinen PR zusammenführen?

yes!

@Paulchen-Panther
Copy link
Member

Danke schön

@Paulchen-Panther Paulchen-Panther merged commit 9110b3e into hyperion-project:master Jul 12, 2020
@SJunkies SJunkies deleted the dbmanager-orderby branch July 20, 2020 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants