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

Extracted content ID by content type fetching as a strategy #330

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Feb 6, 2024

Question Answer
JIRA issue IBX-7149
Type improvement
Target #296, 4.5+
BC breaks no

Proposition of a solution for #296 (comment).

I've extracted fetching content ids for content type a separate strategy. Limiting the refactoring to just that, because it's already a bit too complex.

The end goal would be to extract all of those if-elseif-elseif-else statements into separate strategies following the internal ContentIdListGeneratorStrategyInterface. I'd like to introduce that as a follow-up (maybe for 4.6 only). The strategy is tied to SF CLI InputInterface so it can be used uniformly instead of the mentioned if-elseif.

Every ContentList is cached by its content type so we don't execute it twice to get total count and the list itself. Other strategies won't require it.

Achieving more SOLID code here, while might look slightly over-engineered, it allows to unit-test specific parts of the solution. There are no tests for specific scenarios ibexa:reindex command itself as it's too complicated to be automated. Its basic usage is covered by behat, when installing the Product from scratch.

@alongosz alongosz force-pushed the ibx-7149-reindex-cmd-content-id-gen-strategy branch from fe88536 to d12659e Compare February 6, 2024 21:45
Copy link

sonarqubecloud bot commented Feb 6, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@alongosz alongosz marked this pull request as ready for review February 7, 2024 09:33
@alongosz
Copy link
Member Author

alongosz commented Feb 8, 2024

Incorporating the changes into #296. More chances for review will be there :)

@alongosz alongosz merged commit e596941 into IBX-7149_reindex_problem_with_content_type Feb 8, 2024
23 checks passed
@alongosz alongosz deleted the ibx-7149-reindex-cmd-content-id-gen-strategy branch February 8, 2024 12:07
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.

3 participants