algolia / jekyll-algolia

Add fast and relevant search to your Jekyll site
https://community.algolia.com/jekyll-algolia/
MIT License
214 stars 35 forks source link

Drafts without dates not rendering correctly #177

Closed leviem1 closed 3 years ago

leviem1 commented 3 years ago

I want to report a bug:

What is the current behavior?

In the recent 1.7.0 release, drafts without a date throw the following exception when using --drafts.

/usr/gem/gems/jekyll-3.9.0/lib/jekyll/drops/url_drop.rb:39:in `year': undefined method `strftime' for nil:NilClass (NoMethodError)
Progress: |jekyll 3.9.0 | Error:  undefined method `strftime' for nil:NilClass
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/drops/drop.rb:52:in `public_send'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/drops/drop.rb:52:in `[]'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/url.rb:105:in `block in generate_url_from_drop'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/url.rb:95:in `gsub'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/url.rb:95:in `generate_url_from_drop'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/url.rb:62:in `generate_url'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/url.rb:53:in `generated_url'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/url.rb:39:in `to_s'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/document.rb:211:in `url'
    from /usr/gem/gems/jekyll-algolia-1.7.0/lib/jekyll/algolia/file_browser.rb:217:in `url'
    from /usr/gem/gems/jekyll-algolia-1.7.0/lib/jekyll/algolia/file_browser.rb:157:in `metadata'
    from /usr/gem/gems/jekyll-algolia-1.7.0/lib/jekyll/algolia/extractor.rb:18:in `run'
    from /usr/gem/gems/jekyll-algolia-1.7.0/lib/jekyll/algolia/overwrites/jekyll-algolia-site.rb:118:in `block in push'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/site.rb:332:in `block (2 levels) in each_site_file'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/site.rb:331:in `each'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/site.rb:331:in `block in each_site_file'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/site.rb:330:in `each'
    from /usr/gem/gems/jekyll-3.9.0/lib/jekyll/site.rb:330:in `each_site_file'
    from /usr/gem/gems/jekyll-algolia-1.7.0/lib/jekyll/algolia/overwrites/jekyll-algolia-site.rb:108:in `push'
    from /usr/gem/gems/jekyll-algolia-1.7.0/lib/jekyll/algolia/overwrites/jekyll-algolia-site.rb:37:in `process'
    from /usr/gem/gems/jekyll-algolia-1.7.0/lib/jekyll-algolia.rb:82:in `run'
    from /usr/gem/gems/jekyll-algolia-1.7.0/lib/jekyll/commands/algolia.rb:38:in `block (2 levels) in init_with_program'
    from /usr/gem/gems/mercenary-0.3.6/lib/mercenary/command.rb:220:in `block in execute'
    from /usr/gem/gems/mercenary-0.3.6/lib/mercenary/command.rb:220:in `each'
    from /usr/gem/gems/mercenary-0.3.6/lib/mercenary/command.rb:220:in `execute'
    from /usr/gem/gems/mercenary-0.3.6/lib/mercenary/program.rb:42:in `go'
    from /usr/gem/gems/mercenary-0.3.6/lib/mercenary.rb:19:in `program'
    from /usr/gem/gems/jekyll-3.9.0/exe/jekyll:15:in `<top (required)>'
    from /usr/local/bundle/bin/jekyll:29:in `load'
    from /usr/local/bundle/bin/jekyll:29:in `<main>'

What is your expected behavior?

The current date should be used when generating the url rather than trying to get the date of the file. Not sure how Jekyll does this internally, but we could probably use their approach.

Git repository to reproduce the issue:

Issue Workaround

Ruby version used:

I'm using the jekyll:builder docker image to build my site, so I'm not actually certain, but I doubt it matters here.

Jekyll version used:

3.9.0

Note

Since I was the one that pushed for the inclusion of this feature, didn't do my due diligence in testing it, and it's now affecting my projects, I'll more than gladly lead the way on getting this resolved, rather than just having the feature be reverted.

Haroenv commented 3 years ago

Thanks for suggesting a change. I'm personally more in favour of catching this error and giving a clearer error on where to add a date.

How does it behave for non-draft posts without date?

leviem1 commented 3 years ago

Turns out not including a date in a non-draft post is handled gracefully. There may be some clues there as to how we can avoid this issue, as long as the behavior isn't "just ignore it", since that'd make the drafts invisible again.

Jekyll doesn't require dates for drafts to be rendered, so it would be a bit jarring to force users to start dating their drafts.

leviem1 commented 3 years ago

Hey, just checking in on whether or not you've seen my PR. I assume you're busy so I understand if it's lower priority, but wanted to make sure this didn't fall off the radar.

Thanks for everything!

Haroenv commented 3 years ago

I've released this under 1.7.1, thanks for the PR!