Story/vspc 208 #341

pooja-thalur · 2024-04-19T23:17:34Z

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)

https://diging.atlassian.net/browse/VSPC-208

...Put description here...
On the exhibition configuration page where you can set the exhibition title, etc. There should be three additional settings: scene link icon, module link icon, and external link icon. For each setting the user should be able to upload an image to be used as the default image for all links of that type.

This ticket requires changes on the staff side (to configure the link images) and the public side (use the default image if configured) for "arrow" type links.

Anything else the reviewer needs to know?

... describe here ...

…ternal link image

… end

…ibition

# Conflicts: # vspace/src/main/java/edu/asu/diging/vspace/core/services/IImageService.java

jdamerow · 2024-06-11T19:26:21Z

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

+     *@param image - The image data as a byte array
+     *@param filename - The name of the file to be stored
+     *@return {@link IVSImage} instance 
+     *@throws FileStorageException If there is an error storing the file


is that true? where is it thrown?

jdamerow · 2024-06-11T19:36:30Z

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

+        exhibition = (Exhibition) exhibitManager.storeExhibition(exhibition);
+        attributes.addAllAttributes(addSuccessAttributes(exhibition));
+
+        return new RedirectView(request.getContextPath() + "/staff/exhibit/config");


there should be three different forms to submit the default images; each should call this function with a parameter defining which default image to set; and then map the parameter to a function that sets the default on the image at exhibition.

jdamerow · 2024-06-11T19:36:52Z

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

+     */
+    @RequestMapping(value = "/staff/exhibit/config/link/defaultImage", method = RequestMethod.POST)
+    public RedirectView createOrUpdateLinkImage(HttpServletRequest request,
+            @RequestParam(required = false, name = "exhibitionParam") String exhibitID,


this should not be needed; right now, you can just get the start exhibition

jdamerow · 2024-06-26T14:54:32Z

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

+    public static final String API_DEFAULT_SPACE_IMAGE_PATH = "/api/image/default/link/space/";
+    public static final String API_DEFAULT_MODULE_IMAGE_PATH = "/api/image/default/link/module/";
+    public static final String API_DEFAULT_EXTERNAL_IMAGE_PATH = "/api/image/default/link/external/";
+    public static final String API_DEFAULT_SPACE_IMAGE_STATUS = "/api/exhibition/default/link/image/status/";


API endpoints should be in the api package.

jdamerow · 2024-06-26T14:59:34Z

Make it so, Jenkins.

jdamerow · 2024-06-26T15:00:57Z

There are test failures.

jdamerow · 2024-07-03T19:16:22Z

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

+    }   
+
+    /**
+     * Method to store an image in the file location


what file location? this is confusing. the method only gets an image and a filename, right? nothing about location?

jdamerow · 2024-07-03T19:18:03Z

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

+                relativePath = storage.storeFile(image, filename, storedImage.getId());
+            } catch (FileStorageException e) {
+                logger.error(DEFAULT_IMAGE_EXCEPTION,e);
+                returnValue.getErrorMsgs().add(DEFAULT_IMAGE_EXCEPTION + e.getMessage());


if this is the case and the image can't be stored, the vsimage object should not be stored either.

in line 291 deleting the vsimage object if an exception occurs, as we are using its id to store the image

jdamerow · 2024-07-03T19:19:36Z

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


            exhibition.setCustomMessage(customMessage);
        }
+
+        exhibition = (Exhibition) exhibitManager.storeExhibition(exhibition);


why does this need to be casted to Exhibition here?

because exhibitManager.storeExhibition(exhibition) is returning IExhibition type

yes, but why would it need to be Exhibition here?

jdamerow · 2024-07-03T19:20:02Z

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

+     * @throws IOException if an input or output error occurs
+     */
+    @RequestMapping(value = "/staff/exhibit/config/link/defaultImage", method = RequestMethod.POST)
+    public RedirectView createOrUpdateLinkImage(HttpServletRequest request,


this should be in its own controller

jdamerow · 2024-07-03T19:22:38Z

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

+            attributes.addAttribute("alertType", "danger");
+            attributes.addAttribute("showAlert", "true");
+            attributes.addAttribute("message", "Couldn't save the default image");
+            return new RedirectView(request.getContextPath() + "/staff/exhibit/config");


context path should not be needed here. if the method returns String, you can just return redirect:/path....

jdamerow · 2024-07-03T19:24:22Z

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

+        else if(linkType.equals("external")) {
+            IVSImage externalLinkDefaultImage = imageService.storeImage(image.getBytes(), image.getOriginalFilename());         
+            exhibition.setExternalLinkDefaultImage(externalLinkDefaultImage);
+        }


this is pretty ugly and would get worse if we add more link types. as said before, there should be a map that maps "space", "module",... to IExhibition::setSpace... and then there is no if/else needed here, just getting the right method from the map and apply it.

jdamerow · 2024-07-03T19:25:25Z

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

+    }
+
+    @Test
+    public void test_storeDefaultImage_Error() throws FileStorageException, IOException {


jdamerow · 2024-07-10T20:42:42Z

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

+                relativePath = storage.storeFile(image, filename, storedImage.getId());
+            } catch (FileStorageException e) {
+                logger.error(DEFAULT_IMAGE_EXCEPTION,e);
+                returnValue.getErrorMsgs().add(DEFAULT_IMAGE_EXCEPTION + e.getMessage());


jdamerow · 2024-07-10T20:44:14Z

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

+@Controller
+public class DefaultLinkImageController {
+
+    private static final Map<String, BiConsumer<Exhibition, IVSImage>> imageSetterMap = new HashMap<>();


this should not be static

jdamerow · 2024-07-10T20:44:32Z

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

+    IExhibitionManager exhibitManager;
+
+    @Autowired
+    IImageService imageService;   


those need to be private

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

bnayak4 added 30 commits June 9, 2022 16:22

[VSPC-208] added UI changes to include space link, module link and ex…

7f6dc06

…ternal link image

[VSPC-208] connected backend to frontend

efffdaa

[VSPC-208] code changes to remove csrf token error

bbfe3d8

[VSPC-208] added changes for bean class

96df53e

[VSPC-208] tried creating new form to remove the error

95a03e1

[VSPC-208] added the controller and did cosmetic UI changes

4d434db

[VSPC-208] added controller changes

c69e265

[VSPC-208] used Apache Tika to extract data from the images and store it

c71368c

[VSPC-208] refactored the code

46ba692

[VSPC-208] added changes to save the spaceLinkImage

abbc260

[VSPC-208] added changes to save images with spaceId

301ed4c

[VSPC-208] deleted defaultImageRepository

949999d

[VSPC-208] link changes started

d65016e

[VSPC-208] need to set the bgImage fields

c05d011

[VSPC-208] added changes for the image resize

c02f795

[VSPC-208] image extraction when image not uploaded

782b1f0

[VSPC-208] added changes to make the arrow field visible in the front…

5b24201

… end

[VSPC-208] added changed to print the image in the frontEnd

06936e9

[VSPC-208] added changes after testing the staff side

9cd2331

[VSPC-208] Added changes to remove the images

f9cb545

[VSPC-208] getting images in the front end side

f0afaad

[VSPC-208] hibernate exception removed

ec5b455

[VSPC-208] getting hibernate error

a36c852

[VSPC-208] added changes

e391851

[VSPC-208] removed and changed the code to include it for all the exh…

fff11aa

…ibition

[VSPC-208] added changes

2f00e2c

[VSPC-208] added changes

4870da9

[VSPC-208] adding changes

9f401c0

[VSPC-208] added changes to show imageLink in the UI

18e8fc1

[VSPC-208] added changes

47e5029

jdamerow closed this Jun 10, 2024

[VSPC-208] Merge branch 'develop' into story/VSPC-208

6925d29

# Conflicts: # vspace/src/main/java/edu/asu/diging/vspace/core/services/IImageService.java

pooja-thalur reopened this Jun 10, 2024

pooja-thalur requested a review from jdamerow June 10, 2024 19:16

jdamerow requested changes Jun 11, 2024

View reviewed changes

jdamerow closed this Jun 11, 2024

[VSPC-208] addressed pr comments

e281969

pooja-thalur reopened this Jun 13, 2024

pooja-thalur added 3 commits June 13, 2024 16:36

[VSPC-208] code factor

5154844

[VSPC-208] code factor

2bb0739

[VSPC-208] code cleanup

057b477

pooja-thalur requested a review from jdamerow June 14, 2024 15:19

jdamerow requested changes Jun 26, 2024

View reviewed changes

jdamerow closed this Jun 26, 2024

pooja-thalur added 2 commits June 27, 2024 16:15

[VSPC-208] resolved test cases failure

6436850

[VSPC-208] addressed pr comment

0244148

pooja-thalur reopened this Jun 28, 2024

jdamerow requested changes Jul 3, 2024

View reviewed changes

jdamerow closed this Jul 3, 2024

pooja-thalur added 3 commits July 5, 2024 16:33

[VSPC-208] addressed pr comments

27f9f3e

[VSPC-208] addressed pr comments

50c7cd4

[VSPC-208] mapped link type to its method

6e9de01

pooja-thalur reopened this Jul 10, 2024

jdamerow requested changes Jul 10, 2024

View reviewed changes

jdamerow closed this Jul 10, 2024

[VSPC-208] addressed pr comments

af68d29

pooja-thalur reopened this Sep 6, 2024

Merge branch 'develop' into story/VSPC-208

949abb9

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Story/vspc 208 #341

Story/vspc 208 #341

pooja-thalur commented Apr 19, 2024

jdamerow Jun 11, 2024

jdamerow Jun 11, 2024

jdamerow Jun 11, 2024

jdamerow Jun 26, 2024

jdamerow commented Jun 26, 2024

jdamerow commented Jun 26, 2024

jdamerow Jul 3, 2024

jdamerow Jul 3, 2024

jdamerow Jul 10, 2024

pooja-thalur Jul 12, 2024

jdamerow Jul 3, 2024

pooja-thalur Jul 10, 2024

jdamerow Jul 10, 2024

jdamerow Jul 3, 2024

jdamerow Jul 3, 2024

jdamerow Jul 3, 2024

jdamerow Jul 3, 2024

jdamerow Jul 10, 2024

jdamerow Jul 10, 2024

jdamerow Jul 10, 2024

Story/vspc 208 #341

Are you sure you want to change the base?

Story/vspc 208 #341

Conversation

pooja-thalur commented Apr 19, 2024

Guidelines for Pull Requests

Please provide a brief description of your ticket

Anything else the reviewer needs to know?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdamerow commented Jun 26, 2024

jdamerow commented Jun 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment