Merge pull request #44 from roughike/code-cleanup

Code cleanup

lib/redux/app/app_reducer.dart

@@ -4,11 +4,12 @@ import 'package:inkino/redux/app/app_state.dart';
 import 'package:inkino/redux/event/event_reducer.dart';
 import 'package:inkino/redux/show/show_reducer.dart';
 import 'package:inkino/redux/theater/theater_reducer.dart';
+import 'package:redux/redux.dart';
 
 AppState appReducer(AppState state, dynamic action) {
   return new AppState(
     searchQuery: _searchQueryReducer(state.searchQuery, action),
-    actorsByName: _actorReducer(state.actorsByName, action),
+    actorsByName: actorReducer(state.actorsByName, action),
     theaterState: theaterReducer(state.theaterState, action),
     showState: showReducer(state.showState, action),
     eventState: eventReducer(state.eventState, action),
@@ -23,25 +24,28 @@ String _searchQueryReducer(String searchQuery, action) {
   return searchQuery;
 }
 
-Map<String, Actor> _actorReducer(Map<String, Actor> actorsByName, action) {
-  if (action is ActorsUpdatedAction) {
-    var actors = <String, Actor>{}..addAll(actorsByName);
-    action.actors.forEach((actor) {
-      actors.putIfAbsent(actor.name, () => new Actor(name: actor.name));
-    });
-
-    return actors;
-  } else if (action is ReceivedActorAvatarsAction) {
-    var actorsWithAvatars = <String, Actor>{}..addAll(actorsByName);
-    action.actors.forEach((actor) {
-      actorsWithAvatars[actor.name] = new Actor(
-        name: actor.name,
-        avatarUrl: actor.avatarUrl,
-      );
-    });
-
-    return actorsWithAvatars;
-  }
+final actorReducer = combineTypedReducers<Map<String, Actor>>([
+  new ReducerBinding<Map<String, Actor>, ActorsUpdatedAction>(_actorsUpdated),
+  new ReducerBinding<Map<String, Actor>, ReceivedActorAvatarsAction>(_receivedAvatars),
+]);
+
+Map<String, Actor> _actorsUpdated(Map<String, Actor> actorsByName, action) {
+  var actors = <String, Actor>{}..addAll(actorsByName);
+  action.actors.forEach((actor) {
+    actors.putIfAbsent(actor.name, () => new Actor(name: actor.name));
+  });
+
+  return actors;
+}
+
+Map<String, Actor> _receivedAvatars(Map<String, Actor> actorsByName, action) {
+  var actorsWithAvatars = <String, Actor>{}..addAll(actorsByName);
+  action.actors.forEach((actor) {
+    actorsWithAvatars[actor.name] = new Actor(
+      name: actor.name,
+      avatarUrl: actor.avatarUrl,
+    );
+  });
 
-  return actorsByName;
+  return actorsWithAvatars;
 }

lib/ui/event_details/event_backdrop_photo.dart

@@ -1,11 +1,11 @@
+import 'dart:ui' as ui;
+
 import 'package:flutter/material.dart';
 import 'package:inkino/assets.dart';
 import 'package:inkino/data/models/event.dart';
 import 'package:inkino/utils/widget_utils.dart';
 import 'package:meta/meta.dart';
 
-import 'dart:ui' as ui;
-
 class EventBackdropPhoto extends StatelessWidget {
   EventBackdropPhoto({
     @required this.event,
@@ -34,6 +34,8 @@ class EventBackdropPhoto extends StatelessWidget {
 
   Widget _buildPlaceholderBackground(BuildContext context) {
     return new Container(
+      width: MediaQuery.of(context).size.width,
+      height: height,
       decoration: new BoxDecoration(
         gradient: new LinearGradient(
           begin: Alignment.bottomCenter,
@@ -44,8 +46,6 @@ class EventBackdropPhoto extends StatelessWidget {
           ],
         ),
       ),
-      width: MediaQuery.of(context).size.width,
-      height: height,
       child: new Center(
         child: new Icon(
           Icons.theaters,

lib/ui/event_details/showtime_information.dart

@@ -1,8 +1,10 @@
 import 'package:flutter/material.dart';
 import 'package:inkino/data/models/show.dart';
 import 'package:intl/intl.dart';
+import 'package:meta/meta.dart';
 import 'package:url_launcher/url_launcher.dart';
 
+@visibleForTesting
 Function(String) launchTicketsUrl = (url) async {
   if (await canLaunch(url)) {
     await launch(url);

lib/ui/events/event_grid_item.dart

@@ -17,11 +17,7 @@ class EventGridItem extends StatelessWidget {
       gradient: new LinearGradient(
         begin: Alignment.bottomCenter,
         end: Alignment.topCenter,
-        stops: <double>[
-          0.0,
-          0.7,
-          0.7,
-        ],
+        stops: <double>[0.0, 0.7, 0.7],
         colors: <Color>[
           Colors.black,
           Colors.transparent,

lib/ui/events/event_poster.dart

@@ -1,9 +1,11 @@
 import 'package:flutter/material.dart';
 import 'package:inkino/assets.dart';
 import 'package:inkino/data/models/event.dart';
+import 'package:inkino/utils/widget_utils.dart';
 import 'package:meta/meta.dart';
 import 'package:url_launcher/url_launcher.dart';
 
+@visibleForTesting
 Function(String) launchTrailerVideo = (url) async {
   if (await canLaunch(url)) {
     await launch(url);
@@ -23,6 +25,27 @@ class EventPoster extends StatelessWidget {
   final Size size;
   final bool displayPlayButton;
 
+  BoxDecoration _buildDecorations() {
+    return new BoxDecoration(
+      boxShadow: <BoxShadow>[
+        new BoxShadow(
+          offset: const Offset(1.0, 1.0),
+          spreadRadius: 1.0,
+          blurRadius: 2.0,
+          color: Colors.black38,
+        ),
+      ],
+      gradient: new LinearGradient(
+        begin: Alignment.bottomCenter,
+        end: Alignment.topCenter,
+        colors: <Color>[
+          const Color(0xFF222222),
+          const Color(0xFF424242),
+        ],
+      ),
+    );
+  }
+
   Widget _buildPlayButton() {
     if (displayPlayButton && event.youtubeTrailers.isNotEmpty) {
       return new DecoratedBox(
@@ -39,7 +62,7 @@ class EventPoster extends StatelessWidget {
             icon: new Icon(Icons.play_circle_outline),
             iconSize: 42.0,
             color: Colors.white.withOpacity(0.8),
-            onPressed: () async {
+            onPressed: () {
               var url = event.youtubeTrailers.first;
               launchTrailerVideo(url);
             },
@@ -53,45 +76,31 @@ class EventPoster extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    return new Container(
-      decoration: new BoxDecoration(
-        boxShadow: <BoxShadow>[
-          new BoxShadow(
-            offset: const Offset(1.0, 1.0),
-            spreadRadius: 1.0,
-            blurRadius: 2.0,
-            color: Colors.black38,
-          ),
-        ],
-        gradient: new LinearGradient(
-          begin: Alignment.bottomCenter,
-          end: Alignment.topCenter,
-          colors: <Color>[
-            const Color(0xFF222222),
-            const Color(0xFF424242),
-          ],
-        ),
+    var content = <Widget>[
+      new Icon(
+        Icons.local_movies,
+        color: Colors.white24,
+        size: 72.0,
+      ),
+      new FadeInImage.assetNetwork(
+        placeholder: ImageAssets.transparentImage,
+        image: event.images.portraitMedium ?? 'http://example.com',
+        width: size?.width,
+        height: size?.height,
+        fadeInDuration: const Duration(milliseconds: 300),
+        fit: BoxFit.cover,
       ),
+    ];
+
+    addIfNonNull(_buildPlayButton(), content);
+
+    return new Container(
+      decoration: _buildDecorations(),
       width: size?.width,
       height: size?.height,
       child: new Stack(
         alignment: Alignment.center,
-        children: <Widget>[
-          new Icon(
-            Icons.local_movies,
-            color: Colors.white24,
-            size: 72.0,
-          ),
-          new FadeInImage.assetNetwork(
-            placeholder: ImageAssets.transparentImage,
-            image: event.images.portraitMedium ?? 'http://example.com',
-            width: size?.width,
-            height: size?.height,
-            fadeInDuration: const Duration(milliseconds: 300),
-            fit: BoxFit.cover,
-          ),
-          _buildPlayButton(),
-        ],
+        children: content,
       ),
     );
   }

lib/utils/event_name_cleaner.dart

@@ -6,11 +6,9 @@ class EventNameCleaner {
   /// "Avengers: Infinity War (2D)" -> "Avengers: Infinity War"
   /// "Avengers: Infinity War (2D dub)" -> "Avengers: Infinity War"
   ///
-  /// This is probably one of the most horrible regexes I've ever had to come up with.
-  ///
   /// For more, see test/event_name_cleaner_test.dart.
-  static final RegExp _pattern = new RegExp(
-      r"(\s([23]D$|\(([23]D|dub|orig|spanish|swe).*))");
+  static final RegExp _pattern =
+      new RegExp(r"(\s([23]D$|\(([23]D|dub|orig|spanish|swe).*))");
 
   static String cleanup(String name) {
     var matches = _pattern.allMatches(name);

test/redux/app/app_reducer_test.dart

@@ -0,0 +1,86 @@
+import 'package:inkino/data/models/actor.dart';
+import 'package:inkino/redux/app/app_actions.dart';
+import 'package:inkino/redux/app/app_reducer.dart';
+import 'package:inkino/redux/app/app_state.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group('AppReducer', () {
+    test(
+        'when called with ActorsUpdatedAction, should not modify existing actors',
+        () {
+      var state = new AppState.initial().copyWith(
+        actorsByName: <String, Actor>{
+          'Seth Ladd': new Actor(
+            name: 'Seth Ladd',
+            avatarUrl: 'https://seths-avatar-url',
+          ),
+          'Eric Seidel': new Actor(
+            name: 'Eric Seidel',
+            avatarUrl: 'https://erics-avatar-url',
+          ),
+        },
+      );
+
+      var reducedState = appReducer(
+        state,
+        new ActorsUpdatedAction(<Actor>[
+          new Actor(name: 'Seth Ladd', avatarUrl: null),
+          new Actor(name: 'Eric Seidel', avatarUrl: null),
+          new Actor(name: 'Ian Hickson', avatarUrl: null),
+        ]),
+      );
+
+      expect(
+        reducedState.actorsByName,
+        <String, Actor>{
+          'Seth Ladd': new Actor(
+            name: 'Seth Ladd',
+            avatarUrl: 'https://seths-avatar-url',
+          ),
+          'Eric Seidel': new Actor(
+            name: 'Eric Seidel',
+            avatarUrl: 'https://erics-avatar-url',
+          ),
+          'Ian Hickson': new Actor(
+            name: 'Ian Hickson',
+            avatarUrl: null,
+          ),
+        },
+      );
+    });
+
+    test(
+        'when called with ReceivedActorAvatarsAction, should add urls for actors',
+        () {
+      var state = new AppState.initial().copyWith(
+        actorsByName: <String, Actor>{
+          'Seth Ladd': new Actor(name: 'Seth Ladd', avatarUrl: null),
+          'Eric Seidel': new Actor(name: 'Eric Seidel', avatarUrl: null),
+        },
+      );
+
+      var reducedState = appReducer(
+        state,
+        new ReceivedActorAvatarsAction(<Actor>[
+          new Actor(name: 'Seth Ladd', avatarUrl: 'https://seths-avatar-url'),
+          new Actor(name: 'Eric Seidel', avatarUrl: 'https://erics-avatar-url'),
+        ]),
+      );
+
+      expect(
+        reducedState.actorsByName,
+        <String, Actor>{
+          'Seth Ladd': new Actor(
+            name: 'Seth Ladd',
+            avatarUrl: 'https://seths-avatar-url',
+          ),
+          'Eric Seidel': new Actor(
+            name: 'Eric Seidel',
+            avatarUrl: 'https://erics-avatar-url',
+          ),
+        },
+      );
+    });
+  });
+}

test/test_utils.dart

@@ -1,8 +1,23 @@
 import 'dart:async';
 
+import 'package:flutter/foundation.dart';
+import 'package:flutter/services.dart';
 import 'package:http/http.dart' as http;
 import 'package:http/testing.dart' as http;
-import 'package:flutter/foundation.dart';
+
+/// This replaces the built-in HTTP client with a mocked one. The mocked client
+/// will always return a transparent image.
+///
+/// This is a workaround needed for widget tests that use network images,
+/// otherwise the test will crash.
+///
+/// For more context:
+///
+/// * https://github.com/flutter/flutter/issues/13433
+/// * https://github.com/flutter/flutter_markdown/pull/17
+void mockAllImageResponses() {
+  createHttpClient = createMockImageHttpClient;
+}
 
 ValueGetter<http.Client> createMockImageHttpClient = () {
   return new http.MockClient((http.BaseRequest request) {

test/ui/event_details/event_details_page_test.dart

@@ -1,5 +1,4 @@
 import 'package:flutter/material.dart';
-import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:inkino/data/models/actor.dart';
 import 'package:inkino/data/models/event.dart';
@@ -21,7 +20,8 @@ void main() {
     String lastLaunchedTrailerUrl;
 
     setUp(() {
-      createHttpClient = createMockImageHttpClient;
+      mockAllImageResponses();
+
       showtimeInfo.launchTicketsUrl = (url) => lastLaunchedTicketsUrl = url;
       eventPoster.launchTrailerVideo = (url) => lastLaunchedTrailerUrl = url;
     });

test/ui/events/events_page_test.dart

@@ -1,5 +1,4 @@
 import 'package:flutter/material.dart';
-import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:inkino/data/loading_status.dart';
 import 'package:inkino/data/models/actor.dart';
@@ -45,11 +44,11 @@ void main() {
     EventsPageViewModel mockViewModel;
 
     setUp(() {
+      mockAllImageResponses();
+
       observer = new NavigatorPushObserver();
       mockViewModel = new MockEventsPageViewModel();
       when(mockViewModel.refreshEvents).thenReturn(() {});
-
-      createHttpClient = createMockImageHttpClient;
     });
 
     Future<Null> _buildEventsPage(WidgetTester tester) {