Fix for #1602 Insufficient collection information when submitting an article by im-shubham-vish · Pull Request #3327 · DSpace/dspace-angular

im-shubham-vish · 2024-09-12T06:26:53Z

Description

Insufficient collection information when submitting an article.

Instructions for Reviewers

Sometimes, the collections returned by the collection browser are indistinguishable because their names are identical to other collections and their parent communities.
For example, suppose you have two communities named "Institution A" and "Institution B," each with a subcommunity named "Reports." Both subcommunities have a collection called "Research Reports." If I want to submit an item to "Institution B > Reports > Research Reports," it's impossible to differentiate between the "Reports" subcommunity in "Institution A" and "Institution B" when searching for the "Research Reports" collection.
Expected Behavior
Instead of displaying only the name of the parent community, the interface should display the full hierarchy, similar to how it's done in XMLUI.

List of Changes in this PR:

• Added a method getHierarchicalName in sidebar-search-list-element.component.ts that returns the hierarchical name. Now, parentTitle calls getHierarchicalName instead of getParentTitle.
• Added a method createInstanceFromDSpaceObject responsible for creating an instance of DSpaceObject.
• Removed the maxlines argument from sidebar-search-list-element.component.html.
• Added some CSS to the content class in truncatable-part.component.scss.

Steps to Test and Reproduce the Behavior:

Create two top-level communities named "A" and "B."
Create a subcommunity named "Reports" under each top-level community.
Create a collection named "Research Reports" within each "Reports" subcommunity.
Try to submit an item to "B > Reports > Research Reports" and ensure that the full hierarchical name is displayed.

tdonohue · 2024-09-12T15:53:52Z

@im-shubham-vish : Thanks for the PR. Just a note that this is failing lint formatting checks. You can see those locally by running npm run lint. They are also visible in this PR if you look at the "Files changed" tab. Some of these may be fixed automatically if you run npm run lint-fix

We require all PRs to pass lint formatting checks.

EikLoe · 2024-09-27T11:58:28Z

Hi @im-shubham-vish ,
thanks for you PR! I tested it locally and it worked perfectly. I just noticed, you kept the parentTitle$ variable in the TypeScript file. Is there a reason for it? Because as far as I can see, you don't need it any more or do I miss something?
Also, you might adjust the spec test in the sidebar-search-list-element.component.spec.ts in order to test the hierarchicalTitle$ instead of the parentTitle$.

im-shubham-vish · 2024-12-10T08:33:02Z

Hi @Leano1998,
Sorry for the late reply & thanks for your feedback! You're right we have no use case left for parentTitle$. So I've adjusted all over the possible places to use hierarchicalTitle$ instead of the parentTitle$ including spec test files.

Please recheck once and let me know in case you've any feedback.

tdonohue

@im-shubham-vish : Thanks for this PR. I gave it a test today, and the basic functionality is working for me. I've verified, I see the entire hierarchy in the popup for "New -> Community", "New -> Collection" and "New -> Item". Here's what it looks like to me

The only issues that I have with this PR is the way in which you've duplicated the same test over and over in several of the spec files. I think it'd be much cleaner & smaller if you moved these tests to a single location...and Codecov (our test verification tool) would also likely stop complaining that it cannot find your tests.

If you can get your tests updated, I think this would be ready to merge.

tdonohue · 2025-01-22T16:28:06Z

-describe('JournalIssueSidebarSearchListElementComponent',
-  createSidebarSearchListElementTests(JournalIssueSidebarSearchListElementComponent, object, parent, 'parent title', 'title', '5 - 7'),
-);
+function getExpectedHierarchicalTitle(parentObj: Collection, obj: ItemSearchResult): Observable<string> {


I'm not a fan of the repetition of the same getExpectedHierarchicalTitle() function in every test that is based off sidebar-search-list-element.

Why not move all this logic to the sidebar-search-list-element.component.spec.ts? It seems to me that single spec file could be updated to ensure it now checks properly for the hierarchical title instead of the single object's title. That way you don't have to copy this function into 9 other spec files.

github-actions · 2025-03-11T19:15:22Z

Hi @im-shubham-vish,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

im-shubham-vish · 2025-03-31T11:35:18Z

@tdonohue Thanks for the feedback! I have implemented your suggestions and pushed the changes.

Please review and let me know if you have any further feedback.

tdonohue

@im-shubham-vish : Thanks for the updates. I got back to this PR today. It still works when I test it manually. However, the tests don't appear to be running at all. I don't see the output of the tests when I run npm test, or in the GitHub logs. Are you getting these tests to run?

Overall, this code works. But, I still don't see the tests working properly. It's important to have new & working tests so that this code doesn't break in the future. See my inline comments below.

tdonohue · 2025-04-24T22:03:04Z

-  createSidebarSearchListElementTests(JournalSidebarSearchListElementComponent, object, parent, 'parent title', 'title', '1234, 5678'),
-);
+const expectedHierarchicalTitle = getExpectedHierarchicalTitle(parent, object);
+if (expectedHierarchicalTitle) {


This if clause in all tests appears to be skipping the test. When I run these tests locally (or look at the logs of the tests in GitHub), I cannot find any proof of the "JournalSidebarSearchListElementComponent" test being run below.

Are you sure these tests are running? Could we remove this if statement to prove they work?

tdonohue · 2025-04-24T22:09:00Z

+if (expectedHierarchicalTitle) {
+  expectedHierarchicalTitle.subscribe((hierarchicalTitle: string) => {
+    describe('JournalSidebarSearchListElementComponent', () => {
+      createSidebarSearchListElementTests(JournalSidebarSearchListElementComponent, object, parent, hierarchicalTitle, 'title', '1234, 5678');


Since all of these tests call createSidebarSearchListElementTests, couldn't we move the call to getExpectedHierarchicalTitle() into that existing createSidebarSearchListElementTests method? I don't see why we need to have the if or the subscribe above? Is there something I'm missing here?

…ng an article

im-shubham-vish · 2025-06-19T13:27:33Z

@tdonohue We have incorporated the suggestions. Kindly review and if you have any feedbacks let us know.

PitbaranK · 2025-06-21T08:32:27Z

@tdonohue,
Kindly help us by reviewing and merging the changes in this pull request.
If you have any feedback or suggestions, please feel free to share — we’re happy to connect and make any necessary adjustments.

PitbaranK · 2025-08-18T08:00:17Z

@tdonohue,
Could you please help us understand if we are missing anything, as this PR is still not proceeding further even after incorporating the requested changes?

@EikLoe,
Could you kindly take another look and get back on the same? Reference : #3327 (comment)

PitbaranK · 2025-11-04T07:49:00Z

@tdonohue,
Thanks for the review. We have incorporated all the changes requested #3327 (review). Kindly review and close the change requested.

github-actions · 2025-11-19T22:58:10Z

Hi @im-shubham-vish,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

github-actions · 2026-05-08T21:34:55Z

Hi @im-shubham-vish,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

tdonohue · 2026-05-08T21:46:28Z

Closing as this has been solved now in #5296. Thanks for your contribution @im-shubham-vish to DSpace. Unfortunately, a different approach was accepted into the codebase. If you find any missing features in that approach, you are welcome to submit a follow-up PR.

github-actions Bot assigned im-shubham-vish Sep 12, 2024

tdonohue added bug usability component: submission 1 APPROVAL pull request only requires a single approval to merge labels Sep 12, 2024

im-shubham-vish closed this Dec 10, 2024

im-shubham-vish force-pushed the main branch from f79e97c to dcea3ba Compare December 10, 2024 08:25

im-shubham-vish reopened this Dec 10, 2024

tdonohue requested changes Jan 22, 2025

View reviewed changes

github-actions Bot added the merge conflict label Mar 11, 2025

im-shubham-vish force-pushed the main branch from 355baf7 to e737adb Compare March 31, 2025 10:53

github-actions Bot removed the merge conflict label Mar 31, 2025

tdonohue self-requested a review April 15, 2025 14:20

tdonohue requested changes Apr 24, 2025

View reviewed changes

tdonohue added this to DSpace 10.0 Release May 19, 2025

tdonohue removed this from DSpace 9.0 Release May 19, 2025

tdonohue moved this to 👀 Under Review in DSpace 10.0 Release May 19, 2025

Fix for DSpace#1602 Insufficient collection information when submitti…

da2f6e7

…ng an article

im-shubham-vish force-pushed the main branch from 3a80009 to da2f6e7 Compare June 18, 2025 11:59

Fix for DSpace#1602 Insufficient collection information when submitti…

c9b8322

…ng an article

tdonohue self-requested a review June 23, 2025 16:10

Merge branch 'DSpace:main' into main

6da0292

Merge branch 'DSpace:main' into main

778306c

github-actions Bot added the merge conflict label Nov 19, 2025

im-shubham-vish added 2 commits November 28, 2025 12:55

Resolve Conflicts

b5e88f5

Merge branch 'main' into main

35326ad

github-actions Bot removed the merge conflict label Nov 28, 2025

im-shubham-vish added 4 commits February 25, 2026 17:10

Merge branch 'DSpace:main' into main

c4d17db

Merge branch 'DSpace:main' into main

6c530a2

Update auth.service.spec.ts

726ec9c

Update auth.service.spec.ts

70d95eb

PitbaranK approved these changes Mar 6, 2026

View reviewed changes

Merge branch 'DSpace:main' into main

aa972c6

github-actions Bot added the merge conflict label May 8, 2026

tdonohue closed this May 8, 2026

github-project-automation Bot moved this from 👀 Under Review to ✅ Done in DSpace 10.0 Release May 8, 2026

tdonohue removed this from DSpace 10.0 Release May 15, 2026

Conversation

im-shubham-vish commented Sep 12, 2024 • edited by tdonohue Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Instructions for Reviewers

List of Changes in this PR:

Steps to Test and Reproduce the Behavior:

Uh oh!

tdonohue commented Sep 12, 2024

Uh oh!

EikLoe commented Sep 27, 2024

Uh oh!

im-shubham-vish commented Dec 10, 2024

Uh oh!

tdonohue left a comment

Choose a reason for hiding this comment

Uh oh!

tdonohue Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Mar 11, 2025

Uh oh!

im-shubham-vish commented Mar 31, 2025

Uh oh!

tdonohue left a comment

Choose a reason for hiding this comment

Uh oh!

tdonohue Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tdonohue Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

im-shubham-vish commented Jun 19, 2025

Uh oh!

PitbaranK commented Jun 21, 2025

Uh oh!

PitbaranK commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PitbaranK commented Nov 4, 2025

Uh oh!

github-actions Bot commented Nov 19, 2025

Uh oh!

github-actions Bot commented May 8, 2026

Uh oh!

tdonohue commented May 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

im-shubham-vish commented Sep 12, 2024 •

edited by tdonohue

Loading

PitbaranK commented Aug 18, 2025 •

edited

Loading