Avoid redundant requests for community specific JS and CSS files

[trichoplax]

Is your feature request related to a problem? Please describe. On first load, or on cache expiry, communities with no custom JS or CSS make a redundant request for it. Page rendering is delayed until the response arrives, leaving users on a slow connection with a blank white page until then.

Describe the solution you'd like The HTML should be created conditionally for pages that request custom JS or CSS ([community-name].js and [community-name].css) so that the request is only made for communities for which the files exist.

Describe alternatives you've considered The alternative is to accept the delayed rendering for users on a slow connection on first load, on cache expiry, and when accessing the site through private browsing / incognito mode.

Additional context See discussion on #954.

[trichoplax]

Should the code to send a 204 response instead of a 404 be removed once this issue is closed? This will then result in a Selenium test failure if there is a 404 for a community that does have custom JS or CSS. I'm guessing we would want that to fail, to catch any problem with providing the custom files?

See pull request #962 specifically commit 38f981e1.

[cellio]

How does the code know (without trying to fetch, which is the original problem right?) whether there is community-specific CSS/JS? That is, what does the condition check for? Is community-specific CSS/JS registered with qpixel somewhere, or is adding that part of this work?

[trichoplax]

Without trying to fetch

This is easier than it seems. The HTML is already produced conditionally from a template, with some parts omitted in certain circumstances.

For example, the HTML has some differences based on whether the user is currently signed in. No decision is required from the browser perspective about whether to display the "Sign In" button. When the user is signed in, the HTML that the browser receives from the server simply does not contain the "Sign In" button.

Similarly, the HTML for sites other than Code Golf and Judaism would not contain the code that requests custom CSS/JS.

The template for the "Sign In" button as an example of this: (omitting lines not relevant to this example)

<% unless user_signed_in? %>
  <div class="header--item">
    <%= link_to 'Sign Up', new_user_registration_path, class: 'button is-muted is-filled' %>
    <%= link_to 'Sign In', new_user_session_path, class: 'button is-muted is-outlined' %>
  </div>
<% end %>

The line <% unless user_signed_in? %> means the code is only included when the user is not signed in:

• When the user is signed in, the HTML sent to the browser does not contain this part of the code.
• When the user is not signed in, the HTML sent to the browser looks like this:

  <div class="header--item">
    <a class="button is-muted is-filled" href="/users/sign_up">Sign Up</a>
    <a class="button is-muted is-outlined" href="/users/sign_in">Sign In</a>
  </div>

Registering whether there is community specific CSS/JS

Since introducing or removing custom CSS and JS files is a very rare change, I was thinking that initially we could just refer directly to Code Golf and Judaism (mention them explicitly in the template code).

Having a centralised list of those communities which have specific CSS/JS would be better, so that we only have to change that one place when a site changes between having and not having custom CSS/JS.

Even better again would be for the HTML to be conditional on whether the CSS and JS files exist - then creating or removing the CSS and JS files would be the only change required for communities that switch in future - no HTML changes required after this initial one. I don't know Ruby so I don't know how that would be done, but I imagine it could be done.

[trichoplax]

I've edited the issue description to make it clearer that the HTML is to be created conditionally (initially I described the HTML as being conditional, which makes it sound like a decision is being made on the browser side).

[cellio]

Thanks for the extra information.

I realize that having the community-specific assets checked in here is already an abstraction error, but I would feel sad if the qpixel code, intended for general use, baked in references to specific communities on codidact.com. Besides, what if somebody else ran an instance that happened to have a code-golf community?

Having a (general) way for the code to check -- configuration file, a list or JSON file or something in the assets directory, something else -- seems like it would serve our and others' needs better, but I don't know how hard it is. Can the code read a hypothetical customized_communities.txt file and then have "this name/URL slug/whatever is in there" as the condition? Is there a better way to separate the specific list from the code?

[trichoplax]

Good point about other instances.

As for checking something like customized_communities.txt, the template I linked to already accesses things like the specific site settings (for the logo path) and the user preferences (to decide whether the header is sticky). So although I don't know Ruby, or Rails (someone who does please weigh in here), it looks likely that accessing information from a text file would be achievable.

I don't see why it wouldn't also be possible to bypass having a text file, and simply check the directory to see whether something like meta.css happens to exist. This is all happening on the server side, before the HTML is ever sent to the browser, so I don't see a security issue with that.

[cellio]

Oh, are you saying we already bake specific community references into the code?

I'm confused about the existence check. Isn't the original problem that we're having to wait for the server to say "nope, no custom CSS for this community"? Or is the difference between checking for the existence of the file and actually fetching its contents? I proposed the list in a file because that seemed like something that could be loaded once and then cached, but I don't know much about either Ruby or Javascript, so I'm not qualified to have an opinion on how to implement it. Maybe we should go to chat and/or pull in somebody who knows more than we do.

[trichoplax]

We continued the conversation on Discord

[Taeir]

Your suggestions are possible, but the main question is whether it would be faster/better/convenient/secure. The complexity of the current solution is quite low (always try to load, if not there, just say to the client that it is okay), but does incur some client side loading for these files.

Checking file existence

Checking server side whether the files exist seems simple, but it may actually be quite hard with the way assets are served in codidact communities. On development it is easy, but on the production servers those files are not served through rails at all (as far as I know). Instead there is a config in NGINX which specifically serves these files. In essence, the rails server wouldn't know whether there are files there, only a client could tell by trying to make a request.

Definition in config/txt file

Regarding defining a list of communities that have them in a config file/text file, this is certainly possible. However I would then rather opt for using the existing SiteSettings mechanism such that this can be enabled and disabled without requiring server restarts or physical server access. For example a selection for custom js/css to load and have the templates available. Perhaps this would also allow other communities to use the codegolf leaderboard for example if they wanted to.

Full definition in settings

Perhaps the whole definition of the js/css to server should just be configurable completely in the site settings from the platform. However, this carries some security risk. If admins also have access to the server files, it is equivalent, but if they normally don't this would allow them to do things like track users or collect additional information on them.

Hardcoding

In my opinion, hardcoding is the worst choice out of the options. It is inflexible and hard to change (requires direct code change). I think we already have better options listed above.

[trichoplax]

Checking file existence

Thanks for the extra info on how this works in production. I did not realise that the HTML was served from a different place than the CSS and JS files. If that prevents checking the directory contents then I guess the only way to make this work would be for the server itself to make an HTTP request to find out if the CSS and JS files exist, and then cache that result and use it when constructing the HTML from the templates. I would prefer to avoid that much complexity, and to avoid making more work for the server, so maybe config is better.

Definition in config/txt file

I really like your suggestion of using the site settings mechanism instead of a manually updated config file. Particularly for instances of QPixel other than Codidact, where there might be more demand for switching CSS and JS on and off for different communities as they try things out and settle on what they want.

Full definition in settings

Does this mean being able to type CSS and JS directly into the site settings through the user interface? Yes that does sound dangerous. Even if most admins happened to have access to the server files directly, I would not want to assume the same would be true of other QPixel instances, so I would definitely prefer to assume that server admins are a separate role from site admins/moderators and not necessarily held by the same person.

As a less dangerous way to have more fine grained control, we could have several fixed CSS and JS files available, and site admins could use a list of checkboxes to decide which ones to include (rather than being able to edit the code). That would introduce the need to think carefully about how they interact with each other though, so I think for this particular feature request I would rather see just one CSS file and one JS file available in the site settings as checkboxes.

Hardcoding

I definitely agree with your points about this, plus Monica's point about other instances (it makes the QPixel code too specific to Codidact). I think hardcoding is completely ruled out.