-
Notifications
You must be signed in to change notification settings - Fork 3
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
Story/vspc 213 #302
Story/vspc 213 #302
Changes from 1 commit
5b25410
c3f1f8b
c770d63
959b957
bf91424
cbf9519
f3285d4
7ac5aed
ff7a491
1e883bd
e313921
52e2fd5
4575ca4
f57fa0a
7a8e502
0784a7b
5382226
00cc39f
98a3eb8
07f65c6
00d8754
1c566b7
39b9f8b
bcd81b6
72f033b
dcac0ab
33712c2
550a4ee
4cea3c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
package edu.asu.diging.vspace.core.exception; | ||
|
||
public class LanguageListConfigurationNotFound extends Exception { | ||
|
||
|
||
/** | ||
* | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unless you intend to add javadoc here, remove |
||
private static final long serialVersionUID = 1L; | ||
|
||
public LanguageListConfigurationNotFound() { | ||
super(); | ||
// TODO Auto-generated constructor stub | ||
} | ||
|
||
public LanguageListConfigurationNotFound(String arg0, Throwable arg1, boolean arg2, boolean arg3) { | ||
super(arg0, arg1, arg2, arg3); | ||
// TODO Auto-generated constructor stub | ||
} | ||
|
||
public LanguageListConfigurationNotFound(String arg0, Throwable arg1) { | ||
super(arg0, arg1); | ||
// TODO Auto-generated constructor stub | ||
} | ||
|
||
public LanguageListConfigurationNotFound(String arg0) { | ||
super(arg0); | ||
// TODO Auto-generated constructor stub | ||
} | ||
|
||
public LanguageListConfigurationNotFound(Throwable arg0) { | ||
super(arg0); | ||
// TODO Auto-generated constructor stub | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
import edu.asu.diging.vspace.config.ConfigConstants; | ||
import edu.asu.diging.vspace.config.ExhibitionLanguageConfig; | ||
import edu.asu.diging.vspace.core.data.ExhibitionRepository; | ||
import edu.asu.diging.vspace.core.exception.LanguageListConfigurationNotFound; | ||
import edu.asu.diging.vspace.core.model.IExhibition; | ||
import edu.asu.diging.vspace.core.model.impl.Exhibition; | ||
import edu.asu.diging.vspace.core.model.impl.ExhibitionLanguage; | ||
|
@@ -86,32 +87,47 @@ public IExhibition getStartExhibition() { | |
* @param exhibition | ||
* @param defaultLanguage | ||
* @param languages | ||
* @throws LanguageListConfigurationNotFound | ||
*/ | ||
@Override | ||
public void updateExhibitionLanguages(Exhibition exhibition, List<String> codes, String defaultLanguage) { | ||
public void updateExhibitionLanguages(Exhibition exhibition, List<String> codes, String defaultLanguage) throws LanguageListConfigurationNotFound { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the exception should be a runtime exception and wouldn't need to be declared. |
||
if(CollectionUtils.isNotEmpty(codes)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you turn the if condition around (if codes is empty) you can avoid such a big if block by returning right away. |
||
if(defaultLanguage!=null) { | ||
if(defaultLanguage!=null && !codes.contains(defaultLanguage)) { | ||
codes.add(defaultLanguage); | ||
} | ||
if(CollectionUtils.isNotEmpty(exhibitionLanguageConfig.getExhibitionLanguageList())) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. check this right at the beginning. no need to do anything if there is no exhibition language list. |
||
List<ExhibitionLanguage> exhibitionLanguages = exhibitionLanguageConfig.getExhibitionLanguageList() | ||
.stream().filter(languageMap -> codes.contains(languageMap.get(ConfigConstants.CODE))) | ||
.map(map -> { | ||
ExhibitionLanguage exhibitionLanguage = new ExhibitionLanguage((String) map.get(ConfigConstants.LABEL), | ||
(String) map.get(ConfigConstants.CODE), exhibition); | ||
.stream().filter(languageConfig -> codes.contains(languageConfig.get(ConfigConstants.CODE))) | ||
.map(languageMap -> { | ||
ExhibitionLanguage exhibitionLanguage = new ExhibitionLanguage((String) languageMap.get(ConfigConstants.LABEL), | ||
(String) languageMap.get(ConfigConstants.CODE), exhibition); | ||
|
||
if(exhibitionLanguage.getCode().equalsIgnoreCase(defaultLanguage)) { | ||
exhibitionLanguage.setDefault(true); | ||
} else { | ||
exhibitionLanguage.setDefault(false); | ||
} | ||
return exhibitionLanguage; | ||
}).collect(Collectors.toList()); | ||
|
||
if(CollectionUtils.isNotEmpty(exhibitionLanguages)) { | ||
logger.info("Updating Exhibition with Languages" + codes); | ||
logger.trace("Updating Exhibition with Languages" + codes); | ||
exhibition.getLanguages().addAll(exhibitionLanguages); | ||
} | ||
|
||
// List<ExhibitionLanguage> defaultLanguages= exhibition.getLanguages().stream() | ||
// .filter(language -> language.isDefault() && !language.getCode().equals(defaultLanguage) ) | ||
// .map(languageMap -> {languageMap.setDefault(false); return languageMap ; }) | ||
// | ||
// .collect(Collectors.toList()); | ||
// | ||
// exhibition.getLanguages().removeAll(defaultLanguages); | ||
// exhibition.getLanguages().addAll(defaultLanguages); | ||
|
||
|
||
} | ||
|
||
} else { | ||
logger.error("Language list configuration not available"); | ||
throw new LanguageListConfigurationNotFound("Exhibition Language Configuration not found"); | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
|
||
import edu.asu.diging.vspace.config.ExhibitionLanguageConfig; | ||
import edu.asu.diging.vspace.core.data.SpaceRepository; | ||
import edu.asu.diging.vspace.core.exception.LanguageListConfigurationNotFound; | ||
import edu.asu.diging.vspace.core.factory.impl.ExhibitionFactory; | ||
import edu.asu.diging.vspace.core.model.ExhibitionModes; | ||
import edu.asu.diging.vspace.core.model.IExhibition; | ||
|
@@ -59,6 +60,9 @@ public String showExhibitions(Model model) { | |
if(exhibition.getLanguages() != null ) { | ||
model.addAttribute("mappedLanguages", exhibition.getLanguages() | ||
.stream().map(language -> language.getLabel()).collect(Collectors.toList())); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what are "mappedLanguages"? and why not putting the language list in the model (not just the labels), you probably need them somewhere (or if not, it's just a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mappedLanguages is just used to highlight the already mapped languages to an exhibition, in the selection dropdown. This is to avoid adding any more logic in front end. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, makes sense. It needs a better name though. It's not clear what "mappedLanguages" would be in this context. |
||
model.addAttribute("defaultLanguage",exhibition.getLanguages().stream() | ||
.filter(language -> language.isDefault()).findFirst().orElse(null) ); | ||
|
||
} | ||
} else { | ||
model.addAttribute("exhibition", new Exhibition()); | ||
|
@@ -77,6 +81,7 @@ public String showExhibitions(Model model) { | |
* @param spaceParam | ||
* @param attributes | ||
* @return | ||
* @throws LanguageListConfigurationNotFound | ||
*/ | ||
@RequestMapping(value = "/staff/exhibit/config", method = RequestMethod.POST) | ||
public RedirectView createOrUpdateExhibition(HttpServletRequest request, | ||
|
@@ -86,7 +91,7 @@ public RedirectView createOrUpdateExhibition(HttpServletRequest request, | |
@RequestParam(value = "customMessage", required = false, defaultValue = "") String customMessage, | ||
@RequestParam("exhibitLanguage") List<String> languages, | ||
@RequestParam("defaultExhibitLanguage") String defaultLanguage, | ||
RedirectAttributes attributes) throws IOException { | ||
RedirectAttributes attributes) throws IOException, LanguageListConfigurationNotFound { | ||
|
||
ISpace startSpace = spaceManager.getSpace(spaceID); | ||
|
||
|
@@ -100,7 +105,7 @@ public RedirectView createOrUpdateExhibition(HttpServletRequest request, | |
exhibition.setTitle(title); | ||
exhibition.setMode(exhibitMode); | ||
exhibitManager.updateExhibitionLanguages(exhibition,languages,defaultLanguage); | ||
|
||
if(exhibitMode.equals(ExhibitionModes.OFFLINE) && !customMessage.equals(ExhibitionModes.OFFLINE.getValue())) { | ||
exhibition.setCustomMessage(customMessage); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be a runtime exception and exception class names should end in
Exception