From 3eaf7c9e5a6efcc1ccebbe02e84e79922e5e7531 Mon Sep 17 00:00:00 2001 From: Rob Galanakis Date: Tue, 26 May 2026 21:51:27 -0700 Subject: [PATCH 1/2] Lime trip import: handle invalid time better We are seeing yet another new report format from Lime; I think it's invalid. But in the meantime, just report on rows with invalid times, rather than failing the import entirely, so it doesn't become blocking. --- lib/suma/lime/sync_trips_from_report.rb | 35 +++++++++++++++---- spec/suma/lime/sync_trips_from_report_spec.rb | 10 ++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/lib/suma/lime/sync_trips_from_report.rb b/lib/suma/lime/sync_trips_from_report.rb index f8bc9cce..9c31d45c 100644 --- a/lib/suma/lime/sync_trips_from_report.rb +++ b/lib/suma/lime/sync_trips_from_report.rb @@ -7,7 +7,16 @@ class Suma::Lime::SyncTripsFromReport include Appydays::Loggable - class InvalidReportRow < StandardError; end + class InvalidReportRow < StandardError + attr_reader :cause, :extra + + def initialize(cause, msg, extra) + @cause = cause + @extra = extra + super(msg) + end + end + class UnhandledScenario < StandardError; end DEFAULT_VEHICLE_TYPE = Suma::Mobility::ESCOOTER @@ -147,9 +156,11 @@ def _run_for_report(txt) begin self.import_trip_from_row(row, user_email:) imported_rows += 1 - rescue InvalidReportRow - self.logger.error("lime_report_missing_field") - Sentry.capture_message("Lime trip row missing necessary fields") + rescue InvalidReportRow => e + self.logger.error(e.cause, e.extra) + Sentry.capture_message(e.message) do |scope| + scope.set_extras(e.extra) + end invalid_rows += 1 end end @@ -222,7 +233,13 @@ def import_trip_from_row(row, user_email:) external_trip_id: row.fetch(TRIP_TOKEN), our_cost: parsemoney(row.fetch(COST_TO_SUMA)), } - raise InvalidReportRow if fields.values.any?(&:nil?) + if (empty = fields.select { |_k, v| v.nil? }.map { |k, _v| k }).present? + raise InvalidReportRow.new( + "lime_report_missing_field", + "Lime trip row missing necessary fields", + {missing_fields: empty}, + ) + end return fields end @@ -275,7 +292,13 @@ def parsetime(t) return Time.strptime(trimmed, "%a %b %d %Y %H:%M:%S GMT%z") end datepart, timepart = t.split(" ", 2) - raise ArgumentError, "invalid time: #{t.inspect}" unless timepart && datepart + unless timepart && datepart + raise InvalidReportRow.new( + "lime_report_invalid_time", + "Lime trip row has an invalid time value", + {time_value: t}, + ) + end # Handle 01/31/2020 and 2020/01/31. datefmt = /\d\d\d\d$/.match?(datepart) ? "%m/%d/%Y" : "%Y/%m/%d" diff --git a/spec/suma/lime/sync_trips_from_report_spec.rb b/spec/suma/lime/sync_trips_from_report_spec.rb index 63fb7310..20c71930 100644 --- a/spec/suma/lime/sync_trips_from_report_spec.rb +++ b/spec/suma/lime/sync_trips_from_report_spec.rb @@ -85,6 +85,16 @@ expect(Suma::Mobility::Trip.all).to be_blank end + it "logs if the time format is invalid" do + txt = <<~CSV + TRIP_TOKEN,CONSEQUENCE,START_TIME,END_TIME,START_LATITUDE,START_LONGITUDE,END_LATITUDE,END_LONGITUDE,REGION_NAME,USER_TOKEN,EMAIL_ADDRESS,TRIP_DURATION_MINUTES,TRIP_DISTANCE_MILES,COST_TO_SUMA,UNLOCK_COST,DURATION_COST,COST_PER_MINUTE,LIME_ACCESS_COST,STANDARD_FEE,PERCENT_DISCOUNT_RATE,,,,,, + RK7PCB7HWXRK5,warning,Mon Mar 09 2024 00:08:00 GMT+0100 (Central European Standard Time),46162.333333333336,45.57,-122.68,45.58,-122.69,Portland,UHAYR2RQDV7HF,m1@in.mysuma.org,32,0.79,0,0.5,2.24,0.07,2.74,14.02,80,,,,,,#REF! + CSV + expect_sentry_capture(type: :message, arg_matcher: eq("Lime trip row has an invalid time value")) + described_class.new.run_for_report(txt) + expect(Suma::Mobility::Trip.all).to be_blank + end + it "only looks at the configured vendor configuration" do Suma::Lime.trip_report_vendor_configuration_id = 0 txt = <<~CSV From b80d7c790b58097883804ae7afe952778c6549ca Mon Sep 17 00:00:00 2001 From: Rob Galanakis Date: Fri, 29 May 2026 21:18:49 -0700 Subject: [PATCH 2/2] Improve signup agreement styling - Make the entire agreement text clickable and with a highlighted box sort of look. - Use blue around the checkbox. - Submit is enabled and errors if the box is not checked. --- data/i18n/seeds/en/strings.json | 1 + data/i18n/seeds/es/strings.json | 1 + webapp/src/assets/styles/onboarding.scss | 25 ++++++++++++++++++ webapp/src/components/SignupAgreement.jsx | 32 +++++++++++++++++++---- webapp/src/pages/Start.jsx | 4 ++- 5 files changed, 57 insertions(+), 6 deletions(-) diff --git a/data/i18n/seeds/en/strings.json b/data/i18n/seeds/en/strings.json index ab1fdcab..ccbf361e 100644 --- a/data/i18n/seeds/en/strings.json +++ b/data/i18n/seeds/en/strings.json @@ -15,6 +15,7 @@ "common.add_to_homescreen": "Add to homescreen", "common.add_to_homescreen_intro": "Install suma to your homescreen for an optimized experience.", "common.agree": "Agree", + "common.agree_to_continue": "You must agree to continue.", "common.app": "App", "common.app_name": "Suma", "common.back": "Back", diff --git a/data/i18n/seeds/es/strings.json b/data/i18n/seeds/es/strings.json index 937ebb87..881a0f70 100644 --- a/data/i18n/seeds/es/strings.json +++ b/data/i18n/seeds/es/strings.json @@ -15,6 +15,7 @@ "common.add_to_homescreen": "Agregar a pantalla principal", "common.add_to_homescreen_intro": "Instale suma en la pantalla principal de su teléfono para una experiencia optimizada.", "common.agree": "Aceptar", + "common.agree_to_continue": "Debes aceptar para continuar.", "common.app": "Aplicación", "common.app_name": "Suma", "common.back": "Volver", diff --git a/webapp/src/assets/styles/onboarding.scss b/webapp/src/assets/styles/onboarding.scss index 95c4153c..89ae3bb9 100644 --- a/webapp/src/assets/styles/onboarding.scss +++ b/webapp/src/assets/styles/onboarding.scss @@ -1,3 +1,28 @@ +@import "./_theme"; + +.signup-agreement-component { + background-color: lighten($primary, 35%); + border: lighten($primary,20%) 1px solid; + border-radius: $border-radius-lg; + box-sizing: border-box; + color: $gray-700; + cursor: pointer; + padding: map-get($spacers, 3); + + // to help balance the weird label thing + .form-check-input { + flex: 0 0 auto; + margin-left: 0; + margin-top: 0.2em; + border-color: darken($blue, 5%); + } + + a { + // this needs to be replaced eventually... + color: darken($blue, 5%) !important; + } +} + .onboarding-carousel { > .carousel-indicators { display: none !important; diff --git a/webapp/src/components/SignupAgreement.jsx b/webapp/src/components/SignupAgreement.jsx index abd859f3..06a2ab1d 100644 --- a/webapp/src/components/SignupAgreement.jsx +++ b/webapp/src/components/SignupAgreement.jsx @@ -1,19 +1,41 @@ import { t } from "../localization"; +import FormError from "./FormError.jsx"; import React from "react"; import Form from "react-bootstrap/Form"; -export default function SignupAgreement({ checked, onCheckedChanged, ...rest }) { +export default function SignupAgreement({ + checked, + errors, + register, + onCheckedChanged, + ...rest +}) { + function handleClick() { + onCheckedChanged(!checked); + } return ( -
+
onCheckedChanged(e.target.checked)} aria-label={t("auth.agree_aria_label")} + required + isInvalid={!!errors.agree} + {...register("agree", { + validate: (value) => value === true || t("common.agree_to_continue"), + })} {...rest} + onChange={(e) => onCheckedChanged(e.target.checked)} /> -
- {t("auth.sign_up_agreement", { buttonLabel: t("forms.continue") })} +
+
+ {t("auth.sign_up_agreement", { buttonLabel: t("forms.continue") })} +
+ {errors.agree?.message}} + noMargin + className="mt-2" + >
); diff --git a/webapp/src/pages/Start.jsx b/webapp/src/pages/Start.jsx index 11c8fc1d..f35c6527 100644 --- a/webapp/src/pages/Start.jsx +++ b/webapp/src/pages/Start.jsx @@ -99,6 +99,8 @@ export default function Start() {

@@ -106,7 +108,7 @@ export default function Start() { back primaryProps={{ children: t("forms.continue"), - disabled: submitDisabled.isOn || !agreementChecked, + disabled: submitDisabled.isOn, }} />