From 8fcc9b883477201922a082f0ee992ca13e2f6c48 Mon Sep 17 00:00:00 2001
From: Harald Eilertsen <haraldei@anduin.net>
Date: Thu, 10 Mar 2022 17:44:59 +0100
Subject: Refactor gig import code.

Split into two functions, and drop the nesting of the ifs. Technically
the ifs didn't need to be nested in the first place, since each error
case would break the execution flow using `continue` anyways.

Throw parsing errors instead of having to keep pass all the state into
the function that only need to parse one line.
---
 includes/admin/views/giglog_import_gigs.php | 127 ++++++++++++++--------------
 1 file changed, 64 insertions(+), 63 deletions(-)

diff --git a/includes/admin/views/giglog_import_gigs.php b/includes/admin/views/giglog_import_gigs.php
index e68c07c..c6b6a56 100644
--- a/includes/admin/views/giglog_import_gigs.php
+++ b/includes/admin/views/giglog_import_gigs.php
@@ -55,84 +55,85 @@ if ( !class_exists( 'GiglogAdmin_ImportGigsPage' ) ) {
          *
          * @param array<int, mixed> $file
          */
-        static function process_upload(array $file): void {
-            global $wpdb;
-            $newconcert= [];
+        static function process_upload(array $file): void
+        {
             $fo = new SplFileObject($file['tmp_name']);
-            $importerrors = '';
+            $importerrors = [];
             $rid=0;
+
             foreach ($fo as $line) {
                 $rid++;
                 $line = trim( $line );
-                if ( empty($line) ) {
-                    // Skip empty lines
-                    continue;
-                }
-                $resultArray = explode("\t", $line);
-                //unsure if this is needed, considering we are also checking if each individual important field is missing. But if they are not replaced by tabs, then everything gets shifted so probably best to check if a //value is empty and NOT replaced by tab
-                if (count($resultArray)<6)
-                {
-                    $importerrors.= 'Row '.$rid.' is missing a field!'."<br>";
-                    continue;
-                }
-                else{
-                    //Below only checks if the date field is made of 4-2-2 digits, irregardless of their values. Actual date check is lower
-                    if(  ! preg_match("/\d{4}\-\d{2}-\d{2}/",$resultArray[3]))
-                    {
-                        $importerrors.= 'Row '.$rid.' has invalid date!'.esc_html($resultArray[3])."<br>";
-                        continue;
+                if ( !empty($line) ) {
+                    try {
+                        self::process_line( $line );
                     }
-                    else {
-                        if (empty(trim($resultArray[0]))) {
-                            $importerrors.= 'Row '.$rid.' is missing concert name!'."<br>";
-                            continue;
-                        }
-                        elseif (empty(trim($resultArray[1]))) {
-                            $importerrors.= 'Row '.$rid.' is missing venue name!'."<br>";
-                            continue;
-                        }
-                        elseif (empty(trim($resultArray[2]))) {
-                            $importerrors.= 'Row '.$rid.' is missing city name!' ."<br>";
-                            continue;
-                        }
-                        else {
-                            $condate     = date('Y-m-d', strtotime($resultArray[3]));
-                            if ($condate<date("Y-m-d")) {
-                                $importerrors.= 'Row '.$rid.' has date in the past!' . esc_html($resultArray[3]) . "<br>";
-                                continue;
-                            }
-                            else {
-                                $cname       = trim($resultArray[0]);
-                                $venue       = trim($resultArray[1]);
-
-                                if (is_numeric($venue)) {
-                                    $venue = GiglogAdmin_Venue::get($venue);
-                                }
-                                else {
-                                    $venue = GiglogAdmin_Venue::find_or_create($venue,trim($resultArray[2]));
-                                }
-
-                                $ticketlink  = trim($resultArray[4]);
-                                $eventlink   = trim($resultArray[5]);
-
-                                try {
-                                    GiglogAdmin_Concert::create($cname, $venue->id(), $condate, $ticketlink, $eventlink);
-                                }
-                                catch(Exception $e) {
-                                    $importerrors.= 'Row '.$rid.' is duplicate (or failed due unknown error)!'."<br>";
-                                }
-                            }
-                        }
+                    catch (Exception $e) {
+                        $importerrors[] = "Error importing line {$rid}: {$e->getMessage()}";
                     }
                 }
             }
 
             if (!empty($importerrors)) {
-                echo  ($importerrors);
+                echo implode("<br>", $importerrors);
             }
             else {
                 echo ('All rows imported ok');
             }
         }
+
+        static function process_line( string $line ) : void
+        {
+            $resultArray = explode("\t", $line);
+
+            // unsure if this is needed, considering we are also checking if
+            // each individual important field is missing. But if they are not
+            // replaced by tabs, then everything gets shifted so probably best
+            // to check if a value is empty and NOT replaced by tab
+
+            if (count($resultArray) < 6) {
+                throw new Exception("missing a field");
+            }
+
+            if (! preg_match("/\d{4}\-\d{2}-\d{2}/",$resultArray[3])) {
+                throw new Exception("invalid date: " . esc_html($resultArray[3]));
+            }
+
+            if (empty(trim($resultArray[0]))) {
+                throw new Exception("missing concert name");
+            }
+
+            if (empty(trim($resultArray[1]))) {
+                throw new Exception("missing venue");
+            }
+
+            if (empty(trim($resultArray[2]))) {
+                throw new Exception("missing city");
+            }
+
+            $condate = date('Y-m-d', strtotime($resultArray[3]));
+
+            if ($condate<date("Y-m-d")) {
+                throw new Exception("has date in the past: " . esc_html($resultArray[3]));
+            }
+
+            $cname       = trim($resultArray[0]);
+            $venue       = trim($resultArray[1]);
+
+            if (is_numeric($venue)) {
+                $venue = GiglogAdmin_Venue::get(intval($venue));
+                if (!$venue) {
+                    throw new Exception("invalid venue id: {$venue}");
+                }
+            }
+            else {
+                $venue = GiglogAdmin_Venue::find_or_create($venue,trim($resultArray[2]));
+            }
+
+            $ticketlink  = trim($resultArray[4]);
+            $eventlink   = trim($resultArray[5]);
+
+            GiglogAdmin_Concert::create($cname, $venue->id(), $condate, $ticketlink, $eventlink);
+        }
     }
 }
-- 
cgit v1.2.3