Story/vspc 196 by ajay3568yadav · Pull Request #344 · diging/virtual-spaces-2.0

ajay3568yadav · 2024-06-05T22:18:16Z

Guidelines for Pull Requests

If you haven't yet read our code review guidelines, please do so, You can find them here.

Please confirm the following by adding an x for each item (turn [ ] into [x]).

I have removed all code style changes that are not necessary (e.g. changing blanks across the whole file that don’t need to be changed, adding empty lines in parts other than your own code)
I am not making any changes to files that don’t have any effect (e.g. imports added that don’t need to be added)
I do not have any sysout statements in my code or commented out code that isn’t needed anymore
I am not reformatting any files in the wrong format or without cause.
I am not changing file encoding or line endings to something else than UTF-8, LF
My pull request does not show an insane amount of files being changed although my ticket only requires a few files being changed
I have added Javadoc/documentation where appropriate
I have added test cases where appropriate
I have explained any part of my code/implementation decisions that is not be self-explanatory

Please provide a brief description of your ticket

(you can copy the ticket if it hasn't changed)

It should be possible to reuse link images

Anything else the reviewer needs to know?

https://diging.atlassian.net/browse/VSPC-196?focusedCommentId=24055

diging-jenkins · 2024-06-05T22:18:19Z

Can one of the admins verify this patch?

jdamerow · 2024-06-10T15:59:28Z

please reopen existing pr so the comment history is kept

jdamerow · 2024-11-08T21:02:04Z

Resolve conflicts please

jdamerow

I'll stop here. There seem to be a bunch of wrong formatting and indentation changes, and in general unnecessary changes. Please fix first before I review.

jdamerow · 2024-11-08T21:02:46Z

vspace/pom.xml

-        <javers.version>5.15.0</javers.version>
+        <spring-security-version>5.3.3.RELEASE</spring-security-version>
+        <thymeleaf.version>3.0.11.RELEASE</thymeleaf.version>
+        <javers.version>5.11.1</javers.version>


is there are reason these are being downgraded?

jdamerow · 2024-11-08T21:02:52Z

vspace/pom.xml

+        <db.url>jdbc:mysql://localhost:3306/vspace?serverTimezone=UTC</db.url>
+        <db.user>root</db.user>
+        <db.password>Metroplaza@2023</db.password>
+        <uploaded.files.path>/Users/ajayyadav/Downloads/Vpsace_images</uploaded.files.path>


should not be changed

jdamerow · 2024-11-08T21:03:03Z

vspace/pom.xml

+    		<groupId>javax.xml.bind</groupId>
+    		<artifactId>jaxb-api</artifactId>
+    		<version>2.3.1</version>
+		</dependency>


jdamerow · 2024-11-08T21:03:14Z

vspace/pom.xml

+			<artifactId>thymeleaf-extras-springsecurity5</artifactId>
+			<version>3.0.3.RELEASE</version>
+		</dependency>
+		


jdamerow · 2024-11-08T21:03:35Z

vspace/pom.xml

    </build>
-</project>
+
+</project>


there are a bunch of versions being downgraded here which they shouldn't

vspace/src/main/java/edu/asu/diging/vspace/core/services/impl/ExternalLinkManager.java

jdamerow · 2024-11-08T21:06:17Z

vspace/src/main/java/edu/asu/diging/vspace/core/services/impl/LinkManager.java

+    protected abstract T getTarget(String linkedId);
+
+    protected abstract U createDisplayLink(L link);
+


I'm so confused, why is this here? I this needed for this ticket?

ah I see, this should not be moved

This came from merging changes from the main branch

jdamerow · 2024-11-08T21:08:27Z

vspace/src/main/java/edu/asu/diging/vspace/web/staff/AddModuleLinkController.java

-            @RequestParam("moduleLinkLabel") String moduleLinkLabel, @RequestParam("moduleType") String displayType,
-            @RequestParam("moduleLinkImage") MultipartFile file)
-            throws NumberFormatException, SpaceDoesNotExistException, IOException, ImageCouldNotBeStoredException {
+	public ResponseEntity<String> createModuleLink(@PathVariable("id") String id, @RequestParam("x") String x,


Indentation is wrong I believe

jdamerow · 2025-01-06T16:39:28Z

There are still issues:

[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /var/lib/jenkins/workspace/VSpace_deploy_to_review_on_pr/vspace/src/main/java/edu/asu/diging/vspace/core/services/impl/SpaceManager.java:[318,24] error: method findByName(String) is already defined in class SpaceManager

ajay3568yadav · 2025-01-06T21:27:47Z

fixed this error

jdamerow · 2025-02-07T20:42:55Z

vspace/src/main/resources/exhibitionLanguages.properties

  { "code": "ve", "label": "Venda" },
  { "code": "vi", "label": "Vietlabelse" },
-  { "code": "vo", "label": "Volap�k" },
+  { "code": "vo", "label": "Volap\uFFFDk" },


that seems wrong (as does the change above)

jdamerow · 2025-02-07T20:43:19Z

Make it so, Jenkins.

jdamerow · 2025-02-07T20:48:21Z

There are test failures

jdamerow · 2025-08-06T08:02:37Z

vspace/src/main/java/edu/asu/diging/vspace/core/services/impl/LinkManager.java

+            throws SpaceDoesNotExistException, ImageCouldNotBeStoredException, ImageDoesNotExistException {

+        if (title == null || title.trim().isEmpty()) {
+            throw new IllegalArgumentException("Title cannot be null or empty.");


this is not the right exception to throw; it's a runtime exception so it does not need to be caught and handled, because it would be thrown if it's an argument that simply can't be dealt with. Here, it can definitely be dealt with if the title is null (simply ask the user to enter a title). Also, I don't see why the title couldn't be empty. If the user, for instance, adds an image, there does not need to be title to make this work.

jdamerow · 2025-08-06T08:02:50Z

vspace/src/main/java/edu/asu/diging/vspace/core/services/impl/LinkManager.java

+            throw new IllegalArgumentException("Title cannot be null or empty.");
+        }
+        if (id == null || id.trim().isEmpty()) {
+            throw new IllegalArgumentException("ID cannot be null or empty.");


see above, wrong exception to be thrown.

jdamerow · 2025-08-06T08:03:08Z

vspace/src/main/java/edu/asu/diging/vspace/core/services/impl/LinkManager.java

+            throw new SpaceDoesNotExistException("Linked ID cannot be null or empty.");
+        }
+        if (displayType == null) {
+            throw new IllegalArgumentException("DisplayType cannot be null.");


jdamerow · 2025-08-06T08:03:58Z

vspace/src/main/java/edu/asu/diging/vspace/core/services/impl/LinkManager.java

+            throw new IllegalArgumentException("DisplayType cannot be null.");
+        }
+        if ((linkImage == null || linkImage.length == 0) && (existingImageId == null || existingImageId.trim().isEmpty())) {
+            throw new ImageDoesNotExistException("Either linkImage or existingImageId must be provided.");


wrong exception to throw. Image does not exist is different than no image provided.

jdamerow · 2025-08-06T08:05:01Z

vspace/src/main/java/edu/asu/diging/vspace/core/services/impl/SpaceManager.java

+//        List<ISpace> spaceResults = new ArrayList<>();
+//        spaces.forEach(r -> spaceResults.add(r));
+//        return spaceResults;
+//    }


why is this commented out?

jdamerow · 2025-08-06T08:16:17Z

vspace/src/main/webapp/WEB-INF/views/staff/spaces/space.html

+        	if (typeof(file) != "undefined"){
+        		chooseFromExisting = false;
+        		formData.append('file', file);
+        	} */


jdamerow · 2025-08-06T08:16:23Z

vspace/src/main/webapp/WEB-INF/views/staff/spaces/space.html

+                    $("#module-selectedImage").empty();
+                    $("#module-imageId").val(null).trigger('change');
                    $('#errorAlert').hide();
+                    console.log("Success");


jdamerow · 2025-08-06T08:17:44Z

vspace/src/test/java/edu/asu/diging/vspace/core/services/impl/ExternalLinkManagerTest.java

        Mockito.when(externalLinkDisplayRepo.save((ExternalLinkDisplay)externalDisplayLink)).thenReturn((ExternalLinkDisplay)externalDisplayLink);

-        IExternalLinkDisplay savedExternalLinkDisplay1 = managerToTest.createLink("New External Link", spaceId1, 10, 30, 40, "EXL001", "New External Link", DisplayType.ARROW, null, null,null);
+        IExternalLinkDisplay savedExternalLinkDisplay1 = managerToTest.createLink("New External Link","EXL001", 10.0f, 30.0f, 40, "EXL001", "New External Link", "Description", DisplayType.ARROW, null, null, null, "IMG000000001");


Is the second parameter not the space id? You are replacing the space id with the external link id here.

jdamerow · 2025-08-06T08:32:09Z

vspace/src/test/java/edu/asu/diging/vspace/core/services/impl/SpaceLinkManagerTest.java

+
+        Mockito.when(spaceManager.getSpace(spaceId1)).thenReturn(space);
+        Mockito.when(spaceLinkRepo.save((SpaceLink)spaceLink)).thenReturn((SpaceLink) spaceLink);
+        Mockito.when(spaceManager.getSpace(spaceId1)).thenReturn(space);


duplicate of line 152

jdamerow · 2025-08-06T08:37:20Z

vspace/src/test/java/edu/asu/diging/vspace/core/services/impl/SpaceLinkManagerTest.java

+        Mockito.when(spaceDisplayManager.getBySpace(space)).thenReturn(displayAttributes);
+        Mockito.when(spaceLinkRepo.save((SpaceLink) spaceLink)).thenReturn((SpaceLink)spaceLink);
+        Mockito.when(spaceLinkDisplayRepo.save((SpaceLinkDisplay)spaceLinkDisplay)).thenReturn((SpaceLinkDisplay)spaceLinkDisplayUpdated);
+        Mockito.when(imageService.getImageById(Mockito.anyString())).thenReturn(spcImage);


only if the id of the image is passed in, spcImage should be returned.

jdamerow · 2025-08-06T08:37:54Z

Make it so, Jenkins.

jdamerow · 2025-08-06T08:41:11Z

Tests are failing.

ajay3568yadav added 4 commits May 30, 2024 16:09

VSPC-196 new branch

bc5fd0b

VSPC-196 minor space issue fixed

53ed254

replicating form VSPC-172

0466d24

[VSPC-196] CSS fix for choose from existing

abb5e86

ajay3568yadav closed this Jun 5, 2024

ajay3568yadav added 2 commits June 5, 2024 16:31

VSPC-196 edit module link fixed

8570989

[VSPC-196] fixed edit link

e88f51c

ajay3568yadav reopened this Jun 6, 2024

jdamerow closed this Jun 10, 2024

ajay3568yadav added 12 commits July 2, 2024 16:39

VSPC-196 merge

b4f181d

VSPC-196 added choose from exisitng images option to spaces

83af595

VSPC-196 added choose from existing image to modules

eead7d2

VSPC-196 created custom formatTemplate

f0fb64f

VSPC-196 added choose from exisitng option to the external links

4eb3421

VSPC-196 fixed external module link

8d554ba

VSPC-196 fixed edit links

16e70d5

VSPC-196 external link images not showing

782bc9d

VSPC-196 testing

ac9881b

VSPC-196 edit external links fixed

6181c3c

VSPC-196 removed comments

f438063

VSPC-196 fixed edit module error

a3550c7

ajay3568yadav reopened this Jul 24, 2024

ajay3568yadav added 2 commits August 15, 2024 16:46

VSPC-196 css issue

931c1c7

VSPC-196 undo changes

56ec6cc

ajay3568yadav requested a review from jdamerow November 6, 2024 15:58

jdamerow requested changes Nov 8, 2024

View reviewed changes

jdamerow closed this Nov 8, 2024

jdamerow closed this Jan 6, 2025

VSPC-196 compilation errors fixed

3d1e014

ajay3568yadav reopened this Jan 6, 2025

ajay3568yadav closed this Jan 6, 2025

VSPC-196 added exception handler

628a36d

ajay3568yadav reopened this Jan 9, 2025

ajay3568yadav closed this Jan 9, 2025

VSPC-196 build failur fix added exceptions handlers

6b5abca

ajay3568yadav reopened this Jan 9, 2025

ajay3568yadav closed this Jan 9, 2025

ajay3568yadav reopened this Jan 22, 2025

VSPC-196 build errors fixed

c881484

jdamerow requested changes Feb 7, 2025

View reviewed changes

jdamerow closed this Feb 7, 2025

VSPC-196 review comments

0ee7527

ajay3568yadav reopened this Feb 7, 2025

ajay3568yadav closed this Feb 7, 2025

ajay3568yadav added 2 commits June 20, 2025 11:45

VSPC-196 merge with develop

9a8996e

VSPC-196 bug fix attempt

9a875e6

ajay3568yadav reopened this Jul 11, 2025

VSPC-196 build fail fix

939ee9b

jdamerow requested changes Aug 6, 2025

View reviewed changes

jdamerow closed this Aug 6, 2025

Girik1105 reopened this Feb 13, 2026

		protected abstract T getTarget(String linkedId);

		protected abstract U createDisplayLink(L link);

Conversation

ajay3568yadav commented Jun 5, 2024

Guidelines for Pull Requests

Please provide a brief description of your ticket

Anything else the reviewer needs to know?

Uh oh!

diging-jenkins commented Jun 5, 2024

Uh oh!

jdamerow commented Jun 10, 2024

Uh oh!

jdamerow commented Nov 8, 2024

Uh oh!

jdamerow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdamerow commented Jan 6, 2025

Uh oh!

ajay3568yadav commented Jan 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdamerow commented Feb 7, 2025

Uh oh!

jdamerow commented Feb 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdamerow commented Aug 6, 2025

Uh oh!

jdamerow commented Aug 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants