Add feature #178 - Fix bug #185 #22
Conversation
| import net.codingwell.scalaguice.InjectorExtensions._ | ||
|
|
||
| object InjectorWrapper { | ||
| private lazy val injector = GuicePlayWebTestApp( |
There was a problem hiding this comment.
What is this class used for? I am wondering because it's in the web-ncer project
| @@ -44,6 +44,7 @@ case class DistributionWidgetSpec( | |||
| relativeValues: Boolean = false, | |||
| numericBinCount: Option[Int] = None, // TODO: rename to binCount | |||
| useDateMonthBins: Boolean = false, | |||
| orderByField: Option[Boolean] = None, | |||
There was a problem hiding this comment.
The type Option[Boolean] looks ugly. Switch to Boolean and deliver a migration script in db-update-0.9.1
| @@ -44,6 +44,7 @@ case class DistributionWidgetSpec( | |||
| relativeValues: Boolean = false, | |||
| numericBinCount: Option[Int] = None, // TODO: rename to binCount | |||
| useDateMonthBins: Boolean = false, | |||
| orderByField: Option[Boolean] = None, | |||
There was a problem hiding this comment.
Also the attribute's name orderByField is rather ambiguous. What about something like orderByLabels?
| if (subFilterIdValue) | ||
| displayElements.push("Sub Filter Id: " + subFilterIdValue) | ||
| if (subFilterValue) | ||
| displayElements.push("Sub Filter: " + subFilterValue) | ||
|
|
There was a problem hiding this comment.
Well, this doesn't fix ada-discovery/ada-issues#180. I don't have access to a test instance but, by looking at the code, these two lines clearly just change the display aspect (displayElements) of the widget - instead of the filter's id, its name is shown. As a matter of fact, this has no effect on the filter's representation (its association with a given widget), which is cleared out when a widget is edited.
Good morning Peter,
I did an update to fix these issues:
I remain at your disposal for any further information.
Luca