BalestraPatrick / HomeKitty

A Vapor 3 website to easily browse HomeKit accessories.
https://homekitty.world
MIT License
75 stars 9 forks source link

Vapor 3 #89

Closed kimdv closed 6 years ago

kimdv commented 6 years ago

Hi!

Started converting to Vapor 3, but still a lot to do!

I took my time to refactor some .leaf files because there was a lot of duplicated code.

I also refactored some routes. See manufactor, categories and accessory.

If any questions feel free to ask!

I will continue to work on it as Vapor 3 will continue to develop and more of the needed services update to Vapor 3

TODO

kimdv commented 6 years ago

We are currently making multiple SQL requests when we fetch categories. First all categories, and then accessory count for each category.

This can be made in one request. I have played around with some queries, and maybe this will work

SELECT category.*, (SELECT COUNT(*) FROM accessories a WHERE a.category_id = category.id) AS accessory_count FROM category

kimdv commented 6 years ago

We are doing a lot of the same around the app, fetching categories, fetching manufacturer count etc.

Can we place this somewhere in the app, so it is accessible from all the controllers?

Because if we change some params on the, lets say manufacturer count, then we need to change it multiple places in the app?

BalestraPatrick commented 6 years ago

Hey @kimdv! This is great. I will take a look at this PR this upcoming week since I am a little busy, can you please point your PR to our new vapor3 branch? Master is only for stable release since it's our production environment πŸ‘

kimdv commented 6 years ago

Pointing to Vapor 3 branch!

There is a lot of opgimatimaztions and refactor there can be made.

kimdv commented 6 years ago

Introduced a QueryHelper because a lot of queries was more or less the same. So if we decide to change one filter etc, we only need to do it one place.

kimdv commented 6 years ago

We could use https://github.com/BrettRToomey/Jobs to automatically update categories with accessory count every 24 ours etc

BalestraPatrick commented 6 years ago

@kimdv Can't we simply continue to use the current method which queries the DB to get the number if accessories in each category? I would try to avoid having a cron job to only count the accessories.

kimdv commented 6 years ago

hmm I can revert back. But there is a lot of unnecessary requests to the db I think.

first fetch categories and afterwards fetch accessories count for each category. This can be made more efficient. :)

BalestraPatrick commented 6 years ago

@kimdv I think it's important to always have an up to date DB from the user perspective. The best option to do this would be to have a trigger in the PostgreSQL DB so that every time a new object is added, the appropriate count column is updated.

BalestraPatrick commented 6 years ago

Hey @kimdv! Thanks a lot for your work so far. I will have more time to work on this towards the end of the month. What's the current status of this PR?

kimdv commented 6 years ago

Hi @BalestraPatrick I have had some busy months. I hope I will get some more time now.

Vapor is also more polished now, so hopefully we can finish it πŸ˜„

I will look into it in the follow days/weeks πŸš€

Sent with GitHawk

kimdv commented 6 years ago

Hi @BalestraPatrick !

I have been working on HomeKitty and I think I have reached something we can use.

Please review when you have time, any feedback is appreciated πŸš€

BalestraPatrick commented 6 years ago

Hey @kimdv, thank you so much! I am trying out the project right now but the project is crashing with this message ⚠️ [PostgreSQLError.server.fatal.InitializeSessionUserId: role "test" does not exist].

Did you create any test table or object in your local PostgreSQL instance?

kimdv commented 6 years ago

No, you need to setup enovirement variables in you Xcode project file. They are not using json files anymore.

screen shot 2018-06-23 at 23 25 21
BalestraPatrick commented 6 years ago

I am still seeing an error in the migration:

[psql] [2018-06-24 14:04:05 +0000] SELECT current_setting('server_version') AS "version" []
[ INFO ] Migrating 'psql' database (MigrationConfig.swift:69)
[psql] [2018-06-24 14:04:06 +0000] SELECT COUNT(*) AS "fluentAggregate" FROM "fluent" []
[psql] [2018-06-24 14:04:06 +0000] SELECT * FROM "fluent" ORDER BY "fluent"."batch" DESC LIMIT 1 OFFSET 0 []
⚠️ [PostgreSQLError.decode: Could not decode UUID: 6 (INTEGER).]

Can you enter the HomeKitty slack group so we can better discuss there? :) https://join.slack.com/t/homekitty/shared_invite/enQtMzg2OTk1NTc5NDI3LTYyY2QzYzYwYmIyZTk5Y2YzZjIxOTJjMGIzNDY4MTU3MWU3NWI0NGUwYTEyYzdiOGE1ODg3OTNjZDBkMGRkZDA

BalestraPatrick commented 6 years ago

Hey @kimdv! I now have some time to look more into our Vapor 3 migration. Are you ok if I merge this PR into the vapor3 branch and we can then fix bugs there, and when we feel confident that it's working locally we can push it to staging in order to deploy it to our staging environment (https://homekitty-staging.herokuapp.com)?

kimdv commented 6 years ago

Sure you can merge it! I’m currently on vacation, so I can’t look into it before next week.

Sent with GitHawk

kimdv commented 6 years ago

I saw that Fluent is updated and PostgresFluent will be updates this week or next week. So we can try and see if it can update with the error you had.

Sent with GitHawk

BalestraPatrick commented 6 years ago

@kimdv I was able to fix the first issue with PostgreSQL but I am now encountering a few more. I'll merge this PR and then we can take care of the rest later.