syndicate API including currently RSS and ATOM formatter API

[rbi-aap]

Please review, let me know if any further improvements are required, thanks

here is demo,
https://dev-newsroom.aap.com.au/api/v1/news/syndicate?formatter=atom&start_date=2024-05-20&products=61ba8a307ba0a51c1603c666,64b73fb739b7d303fde3a8cd

[marwoodandrew]

It maybe be nice to have some tests like those in newsroom/features/news_api_atom_features! particularly ensuring the product filtering on RSS works as expected!

[MarkLark86]

@rbi-aap @marwoodandrew Is there a Jira ticket (in the SDAN project) with details?

[marwoodandrew]

@rbi-aap @marwoodandrew Is there a Jira ticket (in the SDAN project) with details?

No he doesn't my bad! I'll create one!

[marwoodandrew]

@rbi-aap, @MarkLark86 I've created SDAN-727 I think covers what we're trying to achieve.

[MarkLark86]

@rbi-aap There's a lot of duplication of code. Could you please pull the RSS/ATOM feed formatting code into a common function, that can be used by both the existing blueprints and this new endpoint.

[MarkLark86]

Also, it might be good to avoid that after_request handling and place in another blueprint if you can, which in tern uses the get_internal function from Eve. If need be, you can create a a small Resource/Service to specifically handle the request params etc, but leave the handling for formatting to the blueprint view itself

[rbi-aap]

Hi Team,
Can you please review the PR ?
please let me know what else i need to add it
the new start ticket is https://sofab.atlassian.net/browse/SDAN-732, thanks

[MarkLark86]

The savePermissions function was moved, but the original commented lines above it have not.

/** Save permissions for a company
 *
 */

[rbi-aap]

update,

[MarkLark86]

This comment for /** Save permissions for a company still is not in the correct place. Please fix the comment location so it is above the function it is referencing

[MarkLark86]

Instead of creating your own caching mechanism, you can use existing cache that we're already using (which is also used in this version of newsroom). get_cached_resource_by_id.

Using the get_cached_resource_by_id means this entire module will no longer be needed

[rbi-aap]

temp keep it as it is a session level caching different to the [get_cached_resource_by_id], probably change it in the later version update to V2

[MarkLark86]

I don't understand the reason here. Why create a new caching mechanism when we already have one that is used in Production for a long time now, and proven to work. I'm not saying this won't work, but we have something robust and very thoroughly tested.

What benefit does having a session level cache vs an app/redis cache?

[rbi-aap]

The get_cached_resource_by_id can work for sure as it has approved, but please review below reasons for this use case. much appreciate your support

Moving from session-based to app-level caching in CompanyFactory would be a big code change and could cause problems.

Session caching keeps sensitive permission data safe in user sessions and gives more control over caching for each admin user different to normal user and its cache

The current setup works well and fits our needs, so it's probably better to stick with it than spend a lot of time changing things. as it is more than a general cache requirements,

thanks and regards