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

Several adjustments needed #277

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daninden
Copy link

@daninden daninden commented Oct 7, 2024

Allow extension of models (remove sealed):

  • ContentItemCreateModel
  • AssetWithRenditionsReference

Allow settable Id (for unittests):

  • ElementMetadataBase

Return null for content not found instead of an exception:

  • ActionInvoker
  • ManagementHttpClient

Allow injection of HttpClient

  • ManagementClient
  • HttpClient

Motivation

Fixes #270
And several changes that allow us to extend the models used, and to not throw Exceptions on items not found (but rather return null instead). This is a breaking change ofcourse, but having to catch Exceptions to check if something exists is very cumbersome.

Checklist

  • Code follows coding conventions held in this repo
  • Automated tests have been added
  • Tests are passing
  • Docs have been updated (if applicable)
  • Temporary settings (e.g. variables used during development and testing) have been reverted to defaults

How to test

If manual testing is required, what are the steps?

- ContentItemCreateModel
- AssetWithRenditionsReference

Allow settable Id (for unittests):
- ElementMetadataBase

Return null for content not found instead of an exception:
- ActionInvoker
- ManagementHttpClient

Allow injection of HttpClient
- ManagementClient
- HttpClient
@daninden daninden requested review from pokornyd and a team as code owners October 7, 2024 11:46
@pokornyd pokornyd self-assigned this Nov 14, 2024
@pokornyd
Copy link
Member

hello and thank you for the PR, apologies for a rather late reply. since this seems to be a recurring request, I'll make this a priority and review the proposed changes. will let you know with any updates, thanks for your patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use the public class/keyword instead of internal to allow customized HttpClient
2 participants