Add multipage conversion (save_in_group) parameter to document conversion #173
Conversation
WalkthroughThe recent updates focus on enhancing the document conversion capabilities within the Uploadcare API. Notably, a new multi-page conversion feature has been introduced through the addition of the Changes
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- CHANGELOG.md (1 hunks)
- api_examples/rest_api/post_convert_document.rb (1 hunks)
- lib/uploadcare/client/conversion/base_conversion_client.rb (1 hunks)
- spec/fixtures/vcr_cassettes/document_convert_convert_multipage_group.yml (1 hunks)
- spec/fixtures/vcr_cassettes/document_convert_convert_multipage_zip.yml (1 hunks)
- spec/fixtures/vcr_cassettes/document_convert_to_multipage.yml (1 hunks)
- spec/support/vcr.rb (1 hunks)
- spec/uploadcare/client/conversion/document_conversion_client_spec.rb (2 hunks)
- spec/uploadcare/entity/conversion/document_converter_spec.rb (2 hunks)
Files skipped from review due to trivial changes (2)
- spec/fixtures/vcr_cassettes/document_convert_convert_multipage_group.yml
- spec/fixtures/vcr_cassettes/document_convert_to_multipage.yml
Additional context used
LanguageTool
CHANGELOG.md
[duplication] ~10-~10: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...toptions. ## 4.4.2 — 2024-05-29 ### Fixed * Fixed theUploadcare::File.remote_copy` meth...
[uncategorized] ~11-~11: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...ruct::EntityError: {url} must be Hash. Now returns a string instead of aFile` en...
[duplication] ~18-~18: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...Converter. ## 4.4.1 — 2024-04-27 ### Added * AddedAWS Rekognition Moderation` Add-On. * ...
[uncategorized] ~27-~27: You might be missing the article “the” here. (AI_EN_LECTOR_MISSING_DETERMINER_THE)
Context: ...ies. ### Fixed * ThrowAuthErrorif current public key or secret key config are emp...
[uncategorized] ~28-~28: You might be missing the article “a” here. (AI_EN_LECTOR_MISSING_DETERMINER_A)
Context: ...een renamed toAkamaiGeneratorto fix typo in class name. ## 4.4.0 — 2024-03-09 ...
[uncategorized] ~28-~28: You might be missing the article “the” here. (AI_EN_LECTOR_MISSING_DETERMINER_THE)
Context: ...med toAkamaiGeneratorto fix typo in class name. ## 4.4.0 — 2024-03-09 ### Break...
[uncategorized] ~106-~106: You might be missing the article “the” here. (AI_EN_LECTOR_MISSING_DETERMINER_THE)
Context: ...0.0 — 2022-12-29 This version supports latest Uploadcare REST API — [v0.7](https://up...
[style] ~129-~129: In American English, abbreviations like “etc.” require a period. (ETC_PERIOD)
Context: ...s mime-type, image (dimensions, format, etc), video information (duration, format, ...
[style] ~129-~129: In American English, abbreviations like “etc.” require a period. (ETC_PERIOD)
Context: ...information (duration, format, bitrate, etc), audio information, etc - Field `met...
[style] ~129-~129: In American English, abbreviations like “etc.” require a period. (ETC_PERIOD)
Context: ...rmat, bitrate, etc), audio information, etc - Fieldmetadatathat includes arbi...
[style] ~135-~135: This phrase is redundant (‘I’ stands for ‘interface’). Use simply “API”. (ACRONYM_TAUTOLOGY)
Context: ...plications ### Added - Add Uploadcare API interface: -Uploadcare::FileMetadata-...
[duplication] ~161-~161: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...tureVerifier## 3.1.1 — 2021-10-13 - FixedUploadcare::File#store- FixedUploadcare::File#delete` ## 3.1.0 — 2...
[uncategorized] ~168-~168: The grammatical number of this noun doesn’t look right. Consider replacing it. (AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
Context: ... an option to add custom logic to large files uploading process ## 3.0.5 — 2021-04-1...
Markdownlint
CHANGELOG.md
88-88: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
89-89: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
97-97: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
98-98: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
102-102: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
115-115: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
116-116: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
117-117: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
118-118: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
119-119: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
120-120: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
121-121: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
122-122: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
123-123: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
124-124: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
128-128: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
129-129: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
130-130: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
131-131: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
135-135: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
136-136: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
137-137: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
138-138: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
139-139: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
143-143: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
147-147: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
148-148: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
152-152: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
156-156: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
157-157: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
161-161: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
162-162: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
166-166: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
167-167: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
168-168: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
172-172: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
173-173: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
177-177: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
178-178: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
179-179: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
183-183: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
187-187: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
188-188: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
192-192: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
193-193: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
194-194: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
195-195: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
196-196: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
202-202: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
206-206: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
207-207: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
208-208: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
209-209: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
210-210: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
136-136: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
137-137: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
47-47: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
60-60: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
67-67: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
5-5: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
10-10: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
13-13: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
18-18: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
22-22: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
44-44: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
6-6: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
11-11: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
14-14: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
19-19: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
23-23: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
45-45: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
107-107: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
Additional comments not posted (5)
spec/support/vcr.rb (1)
16-16: Good integration with RSpec metadata.Adding
configure_rspec_metadata!enhances the use of VCR with RSpec, leveraging metadata to manage test cassettes effectively. This is a beneficial change for maintaining clean and isolated test environments.lib/uploadcare/client/conversion/base_conversion_client.rb (1)
53-54: Correct implementation of multi-page conversion options.The addition of
storeandsave_in_groupoptions in thebuild_body_for_manymethod is correctly implemented. These changes are essential for supporting the new functionality of multi-page document conversion.spec/uploadcare/client/conversion/document_conversion_client_spec.rb (1)
30-44: Enhanced test coverage for multi-page conversion.The addition of new contexts with specific VCR tags for testing multi-page conversion is well-executed. These tests appropriately cover the new functionality, ensuring that the feature behaves as expected under different scenarios.
spec/uploadcare/entity/conversion/document_converter_spec.rb (1)
Line range hint
11-85: Review of the multipage conversion feature inDocumentConvertertests.The test suite for the
DocumentConverterreflects the new multipage conversion feature well. The use of shared examples to test both single and multipage scenarios is a good practice as it ensures code reusability and maintainability. However, ensure that thevcrtags correspond to the correct VCR cassettes to guarantee that the tests are reliable and reflect real API interactions.CHANGELOG.md (1)
5-7: Changelog Update for Multipage Conversion Feature.The changelog entry for the new
save_in_groupfeature in theDocumentConverter#convertmethod is clear and concise. This will help users understand the new capabilities of the library. Ensure that the changelog is kept up-to-date with all such significant changes to maintain clarity for the end users.Tools
Markdownlint
5-5: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
6-6: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
Description
Add multipage conversion (save_in_group) parameter to document conversion
Checklist
Summary by CodeRabbit
New Features
save_in_groupparameter).Tests