mitydigital / statamic-feedamic

An Atom and RSS feed for Statamic 3, 4 and 5
https://docs.mity.com.au/feedamic
8 stars 9 forks source link

Error on collections without a Date field #29

Open steveparks opened 1 month ago

steveparks commented 1 month ago

Bug description

I'm successfully using Feedamic to create multiple feeds of different collections on my Statamic 5.x site. (thanks!)

But I've just hit a problem with creating a feed of a structured collection that doesn't have a Date field.

When navigating to this feed I get an error 'Object of class Illuminate\Support\Carbon could not be converted to int'.

On investigation, I think it's the lack of a Date field that is causing the problem.

In the Feedamic controller it requires a date field to sort the entries rather than sorting by the system updated_at metadata.

This may well be by design rather than a bug.

But perhaps there could be some minor docs improvements to mitigate this?

  1. In the docs at https://docs.mity.com.au/feedamic/configuration/feeds at the end of the 'Collections' section, could be added "IMPORTANT: This means all collections to be included in feeds must be configured with Publish Dates enabled."

  2. In the comments in feedamic.php could add as line 49: "Each collection must be configured with Publish Dates enabled."

Long term, maybe it'd be desirable to sort by updated_at instead when a date field isn't found?

Steps to reproduce

  1. Install Statamic 5.x, and Feedamic
  2. Create a collection, don't add a Date field
  3. Create a feed, specifying this collection.
  4. Navigate to the feed in a browser

Environment and versions

Environment
Application Name: Convivio
Laravel Version: 11.20.0
PHP Version: 8.3.9
Composer Version: 2.7.7
Environment: local
Debug Mode: ENABLED
URL: convivio.test
Maintenance Mode: OFF
Timezone: Europe/London
Locale: en

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: CACHED

Drivers
Broadcasting: log
Cache: file
Database: sqlite
Logs: stack / single
Mail: smtp
Queue: sync
Session: file

Livewire
Livewire: v3.5.4

Statamic
Addons: 11
Sites: 1
Stache Watcher: Enabled
Static Caching: Disabled
Version: 5.19.0 PRO

Statamic Addons
goldnead/statamic-toc: 1.5
jonassiewertsen/statamic-livewire: 3.6.0
mitydigital/feedamic: 2.4.1
statamic/wikilinks: 2.1.0
stillat/relationships: 2.2.1
studio1902/statamic-peak-browser-appearance: 3.5.0
studio1902/statamic-peak-commands: 8.4.2
studio1902/statamic-peak-seo: 8.15.3
studio1902/statamic-peak-tools: 6.3.1
tv2regionerne/statamic-passport: 1.4.1
tv2regionerne/statamic-private-api: 1.15.0

Additional details

Thanks for a great addon!

steveparks commented 1 month ago

I've confirmed that enabling the Date field on the Collection then makes the feed work for that collection.

martyf commented 1 month ago

The docs change is straight forward from my end (I've not open sourced that site) to reference dated - I guess I assumed it.

I don't feel changing it to use the "updated_at" would make sense as it would change the order of your feed.. but I guess the question is, what is the business case you have for not having a dated collection? i.e. what is it that you're trying to achieve by not having a date field? Just wanting to understand the end goal first.

steveparks commented 1 month ago

It's less that I was deliberately avoiding having a date field for a strong reason, and more that it's an option when creating collections, and on this collection I had no need for one.

It's a structured collection, so will always be shown in the structure, rather than sorted by date order. I don't need/want a date shown on the page. And I don't need scheduled publishing. So when presented with the option I simply chose not to have the publish date on the collection.

Now I've found it's needed for feedamic, I've added a date field. So my specific issue is fixed, I'm just feeding back user experience for improvements.

So, a level 1 improvement would be simply making clear a collection needs a date field. It'd be fine to just do that.

A level 2 improvement would be having a graceful fallback in case a collection doesn't have a date field (as it is just a tick box option on the collection config page in core).

It wouldn't matter if that graceful fallback was less ideal. It's just that it's better than a confusing error.

And, as it happens, in my use case sorting by updated_at would work well, as the content is a knowledge base, and I want a feed of the new and updated content.

martyf commented 1 month ago

That last sentence is the key one here I think.

While it is intended for dated - and can be resolved by having a date(time) on your Entries - I also see what you're trying to do too.

My argument for forcing dated collections is that if you literally need to change one letter of a typo in an old entry you may not necessarily want that to be "updated" content in your feed.

Where as when it is set on an explicit date(time) field, it's a conscious decision to change its order in the feed.

Do you see those cases of a single typo change (for example) updating your feed being an issue or not? Or do you think that an explicit date field makes more sense for the user?

Basically I'm fence sitting at the moment and happy to hear more about how it could be used to help drive change - but just learning more about those use cases.

steveparks commented 1 month ago

Yes, in my case, I wouldn't have minded articles popping back up the feed just for very minor changes. It's a big collection of content and resurfacing some of it from time to time is useful.

However, I do see that it may not be desired in many use cases.

As I say, it may be enough to stop at a level 1 improvement of simply documenting the date requirement.

But given that not having a date on a collection is a simple tickbox option in core, it might be more user friendly to have a graceful fallback, even if it's sub-optimal - with the compromises involved being documented - rather than just causing an error that's not very descriptive.

martyf commented 1 month ago

I'm keeping it as dated for the moment, and have updated the docs. But have tagged this issue for 3.x to include then.