Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error on collections without a Date field #29

Open
steveparks opened this issue Aug 6, 2024 · 6 comments
Open

Error on collections without a Date field #29

steveparks opened this issue Aug 6, 2024 · 6 comments
Labels
3.x enhancement New feature or request

Comments

@steveparks
Copy link
Contributor

steveparks commented Aug 6, 2024

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
Copy link
Contributor Author

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

steveparks added a commit to steveparks/statamic-feedamic that referenced this issue Aug 6, 2024
@martyf martyf added bug Something isn't working enhancement New feature or request labels Aug 8, 2024
@martyf
Copy link
Contributor

martyf commented Aug 8, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

martyf commented Aug 9, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

martyf commented Aug 11, 2024

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.

@martyf martyf added 3.x and removed bug Something isn't working labels Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants