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

[Vocabularies] Create new async Vocabularies resource, service & REST… #2816

Open
wants to merge 3 commits into
base: async
Choose a base branch
from

Conversation

jerome-poisson
Copy link
Contributor

… API.

SDESK-7468

@jerome-poisson
Copy link
Contributor Author

Not fully finished, I have a few points that I need to check (see FIXME).

Copy link
Contributor

@MarkLark86 MarkLark86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple more tasks that are left here, such as creating the module, registering the resource, disabling the REST API from old resource service, and updating the default_settings.MODULE.

See the docs or this PR for an example

Copy link
Contributor

@MarkLark86 MarkLark86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Model looks good to me, just a couple improvement suggestions

superdesk/vocabularies/async_vocabularies.py Outdated Show resolved Hide resolved
superdesk/vocabularies/async_vocabularies.py Outdated Show resolved Hide resolved
superdesk/vocabularies/async_vocabularies.py Outdated Show resolved Hide resolved
superdesk/vocabularies/async_vocabularies.py Outdated Show resolved Hide resolved
- Moved to `vocabularies_async` and `types` to be consistent with `dev_async`.
- Set `internal_resource` to the old resource.
- Added `ResourceConfig`.
- Added module to `default_settings`.
- Addressed feedbacks.
@jerome-poisson
Copy link
Contributor Author

@MarkLark86 thanks for your feedbacks, I should have addressed all of them, let me know if it's fine now.

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

Successfully merging this pull request may close these issues.

2 participants