neocities / neocities-ruby

The Neocities Gem - A CLI and library for using the Neocities web site API.
https://neocities.org
51 stars 14 forks source link

Allow specifying preview file when pushing #51

Open wayword-dev opened 3 months ago

wayword-dev commented 3 months ago

I've tested this functionality with --dry-run but I haven't tested with a live site to ensure that file upload order impacts preview order. I believe it does based on the following lines:

  1. API /upload calls Site.store_file https://github.com/neocities/neocities/blob/0531a1d85fd2b0d7b1fa333669230f867dd83f89/app/api.rb#L130C26-L130C37
  2. Site.store_file calls SiteChange.record with the name of the uploaded file https://github.com/neocities/neocities/blob/0531a1d85fd2b0d7b1fa333669230f867dd83f89/models/site.rb#L1809
  3. SiteChange.record only creates an Event once every 24 hours, so only the first upload will trigger this https://github.com/neocities/neocities/blob/0531a1d85fd2b0d7b1fa333669230f867dd83f89/models/site_change.rb#L16-L19
  4. Events are used on both /site/:sitename and on activity feeds https://github.com/neocities/neocities/blob/0531a1d85fd2b0d7b1fa333669230f867dd83f89/models/site.rb#L1383-L1388 https://github.com/neocities/neocities/blob/0531a1d85fd2b0d7b1fa333669230f867dd83f89/models/site.rb#L1390-L1397

Granted, this isn't going to be perfect and it's going to be dependent on what order the site change files are returned here:

https://github.com/neocities/neocities/blob/0531a1d85fd2b0d7b1fa333669230f867dd83f89/models/site_change.rb#L9-L11

But I believe it should usually follow insertion order.

I'll try and test when if I remember when I next update my site, but if someone with a working test env could check for me that would be much appreciated ^.^

wayword-dev commented 3 months ago

Updated my site and apparently my theory was totally off base here :( Leaving open in case anyone can explain to me what I'm missing, but without that I have to assume postgres is just not returning rows in insertion order, which I guess makes sense given how massive the neocities DB probably is.

Can be closed once a maintainer sees this.

kyledrake commented 3 months ago

It's probably possible to change this. What ordering do you think would make sense for a preview to be the first one? I got an email recently (was it you?) that suggested it should be the last edited one.

wayword-dev commented 3 months ago

Not my email, but I agree with whoever emailed you -- I think doing it from most recently edited to least recently edited makes more sense than what I was thinking. My original thought to make the first file the preview file because activity feed entries felt like they should be immutable to me.

Would just be a matter of changing this to something like (untested)

  def site_change_filenames(limit=4)
    site_change_files_dataset.select(:filename, :created_at).limit(limit).all
                             .partition { |site_change_file| site_change_file.filename.match('html') }
                             .map { |site_change_files| site_change_files.sort_by(&:created_at).reverse }
                             .flatten.map(&:filename)
  end

And then I could update this PR to upload the preview file last instead of first.

Side note, but it feels somewhat strange to allow the preview file ordering to change after the activity feed entry has been created. Activity feed entries just feel to me like they should be immutable (or maybe immutable after a short grace period), but with the 24 hour gap between entries I suppose a single "site edit" / activity feed entry contains all the changes in that 24 hour period and so it is mutable until the period is over.

kyledrake commented 3 months ago

I went ahead and changed it in https://github.com/neocities/neocities/commit/141bba6d62e046141eb201f067a10653dbce2fce, try your change now and let me know if it works. I also changed it to bring 6 back.

It might make more sense to just have a list of all the site files changed in an equal thumbnail size if there's more than one, because then I could show a lot more and that would be cool.

wayword-dev commented 3 months ago

Ah, that sounds awesome, I like the idea of a showing more files for larger changesets a lot! It's interesting that sort_by preserves the created_at ordering from SQL, the docs say the ordering is "indeterminate" and "unstable" but manual testing implies that it preserves ordering when elements are equal. Adding an updated_at column could also help for cases in which someone edits file a.html, b.html, c.html, d.html, e.html, then a.html again.

Not sure what the policy is on having multiple accounts, but it's hard to test with only one account since I only get one test every 24 hours :P However since this should go into effect retroactively and since Dir.glob seems (on my machine) to go through files alphabetically and since I upload files with the CLI, I can see my recent changes in reverse-alphabetical order which implies to me that this worked?