Open jonsgreen opened 2 years ago
This seems like a fine solution although it moves the pattern-matching out of postgres and into memory, right? This might not be a huge deal if caching is utilized properly and given the low level of traffic. Consider not-fixing this (in spite of my previous concern).
@adarsh Yes, this would change the constraint so that the matching does not depend on a database query on every page request. It is true that caching would eliminate any performance concern for requests to the websites. The main issue would be for the speakers and organizers interacting with the rest of CFP app since that part cfp-app will not be cached. On the other hand, the performance penalty is small and maybe not as critical for those use cases.
I will say that another benefit from this PR is that that I think the integration specs for domains would be better implemented using 'lvh.me' since it better reflects what would happen in production.
I guess I felt responsible to at least try a different approach since the current one is considered to be a bad practice. I will leave it to you to decide whether to merge this or put it aside for now.
Reason for Change
Recently while watching this GoRails video on Domain and Subdomain Constraints I noticed that it was recommended to not do database requests in your routes constraints since it means that every request will need to make the request.
It occurred to me that we could avoid the requests entirely by instead matching against the hosts for the events side of cfp-app. It means the environment variable needs to be set for most of cfp app to be accessible but that should not change much once it is done.
Changes