Skip to content

Conversation

ajnyga
Copy link
Contributor

@ajnyga ajnyga commented Aug 31, 2025

No description provided.

Copy link
Member

@touhidurabir touhidurabir left a comment

Choose a reason for hiding this comment

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

Hi @ajnyga , added some initial comments, nothing critical . But I am unable to test the PR locally as it seems bit behind the current state , Need a rebase with latest version . I will do more detail review once it's rebased and tests are passing(as failing in unit test) .

BTW on thing we need some decide on retrieving the specific key like name again from DB or directly from the publication object to reduce DB calls .

@ajnyga ajnyga force-pushed the f11557 branch 2 times, most recently from 4b03250 to d2d6780 Compare October 2, 2025 20:07
@ajnyga
Copy link
Contributor Author

ajnyga commented Oct 2, 2025

I think the failing unit tests have something todo with using the publication object to fetch the keywords and subjects in dc11?

@ajnyga
Copy link
Contributor Author

ajnyga commented Oct 3, 2025

I noticed now that I missed one direct call to the getBySymbolic in classes/search/ArticleSearch.php

However, I changed the new logic for fetching data from the publication object to other places and if the approach is ok I can add it to ArticleSearch as well.

The unit test like I mention above is probably due to the way the test is written. I am not that familiar with the unittests part of the application though. The dc11 itself seems to work ok when you look for example the oai endpoint. I will study the unittests more carefully and will try to suggest a new implementation for that.

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.

2 participants