From 4c7a461683904a24d048c19d26315eb0365db72f Mon Sep 17 00:00:00 2001 From: John Hatton Date: Tue, 14 Aug 2018 17:32:34 -0600 Subject: [PATCH] simplify date handling Now we just try to quickly get the date into a yyyy-mm-dd format, and don't mess with it after that. Had to change tsconfig settings and how commonjs modules are imported in order to get both jest and the app to both be happy with moment. See https://github.com/aurelia/skeleton-navigation/issues/606 --- .../ConfirmDeleteDialog.tsx | 2 +- app/components/FileList.tsx | 2 +- app/components/export/ExportDialog.tsx | 2 +- app/components/people/person/MugShot.tsx | 2 +- .../project/CreateProjectDialog.tsx | 2 +- app/model/Folder.ts | 11 +- app/model/Project/ReadProject.spec.ts | 57 ++++++++ app/model/field/Field.ts | 47 ++++--- app/model/field/FieldSet.ts | 12 +- app/model/file/File.ts | 128 ++++++++++++------ tsconfig.json | 2 +- 11 files changed, 189 insertions(+), 78 deletions(-) create mode 100644 app/model/Project/ReadProject.spec.ts diff --git a/app/components/ConfirmDeleteDialog/ConfirmDeleteDialog.tsx b/app/components/ConfirmDeleteDialog/ConfirmDeleteDialog.tsx index af68976d..5802e46c 100644 --- a/app/components/ConfirmDeleteDialog/ConfirmDeleteDialog.tsx +++ b/app/components/ConfirmDeleteDialog/ConfirmDeleteDialog.tsx @@ -1,7 +1,7 @@ import * as React from "react"; import * as fs from "fs"; import * as Path from "path"; -import * as ReactModal from "react-modal"; +import ReactModal from "react-modal"; import "./ConfirmDeleteDialog.scss"; import CloseOnEscape from "react-close-on-escape"; import { locate } from "../../crossPlatformUtilities"; diff --git a/app/components/FileList.tsx b/app/components/FileList.tsx index bbb2775a..5ad75a92 100644 --- a/app/components/FileList.tsx +++ b/app/components/FileList.tsx @@ -5,7 +5,7 @@ import { computed } from "mobx"; import { observer, Observer } from "mobx-react"; import { Folder } from "../model/Folder"; import { File } from "../model/file/File"; -import * as Dropzone from "react-dropzone"; +import Dropzone from "react-dropzone"; import { remote } from "electron"; const moment = require("moment"); import { Dictionary } from "typescript-collections"; diff --git a/app/components/export/ExportDialog.tsx b/app/components/export/ExportDialog.tsx index 6f7a48d9..82a14c83 100644 --- a/app/components/export/ExportDialog.tsx +++ b/app/components/export/ExportDialog.tsx @@ -1,5 +1,5 @@ import * as React from "react"; -import * as ReactModal from "react-modal"; +import ReactModal from "react-modal"; import "./ExportDialog.scss"; import CloseOnEscape from "react-close-on-escape"; import CsvExporter from "../../export/CsvExporter"; diff --git a/app/components/people/person/MugShot.tsx b/app/components/people/person/MugShot.tsx index 59e6ad03..0e3df5e0 100644 --- a/app/components/people/person/MugShot.tsx +++ b/app/components/people/person/MugShot.tsx @@ -1,7 +1,7 @@ import * as React from "react"; import { observer } from "mobx-react"; import { Person } from "../../../model/Project/Person/Person"; -import * as Dropzone from "react-dropzone"; +import Dropzone from "react-dropzone"; import * as fs from "fs-extra"; import { MugshotPlaceholder } from "./MugshotPlaceholder"; import ImageField from "../../ImageField"; diff --git a/app/components/project/CreateProjectDialog.tsx b/app/components/project/CreateProjectDialog.tsx index fb513aa6..52b77e36 100644 --- a/app/components/project/CreateProjectDialog.tsx +++ b/app/components/project/CreateProjectDialog.tsx @@ -1,7 +1,7 @@ import * as React from "react"; import * as fs from "fs"; import * as Path from "path"; -import * as ReactModal from "react-modal"; +import ReactModal from "react-modal"; import "./CreateProjectDialog.scss"; const { app } = require("electron").remote; diff --git a/app/model/Folder.ts b/app/model/Folder.ts index e3e24435..74e72cbe 100644 --- a/app/model/Folder.ts +++ b/app/model/Folder.ts @@ -12,20 +12,23 @@ import * as fs from "fs-extra"; import * as Path from "path"; import * as glob from "glob"; import { FieldSet } from "./field/FieldSet"; -import * as assert from "assert"; +import assert from "assert"; import ConfirmDeleteDialog from "../components/ConfirmDeleteDialog/ConfirmDeleteDialog"; const sanitize = require("sanitize-filename"); import { trash } from "../crossPlatformUtilities"; export class IFolderSelection { - @observable public index: number; + @observable + public index: number; } // Project, Session, or Person export abstract class Folder { public directory: string = ""; - @observable public files: File[] = []; - @observable public selectedFile: File | null; + @observable + public files: File[] = []; + @observable + public selectedFile: File | null; public metadataFile: File | null; protected safeFileNameBase: string; diff --git a/app/model/Project/ReadProject.spec.ts b/app/model/Project/ReadProject.spec.ts new file mode 100644 index 00000000..f9da689d --- /dev/null +++ b/app/model/Project/ReadProject.spec.ts @@ -0,0 +1,57 @@ +import { ProjectMetadataFile } from "../Project/Project"; +import * as temp from "temp"; +import fs from "fs"; +import Path from "path"; + +let projectDirectory; +let projectName; + +describe("Project Read", () => { + beforeEach(async () => { + projectDirectory = temp.mkdirSync("test"); + projectName = Path.basename(projectDirectory); + }); + afterEach(async () => { + temp.cleanupSync(); + }); + it("should read title", () => { + const f = GetProjectFileWithOneField("Title", "This is the title."); + expect(f.getTextProperty("title")).toBe("This is the title."); + }); + it("empty date should just be empty string", () => { + const f = GetProjectFileWithOneField("DateAvailable", ""); + expect(f.properties.getDateField("dateAvailable").asISODateString()).toBe( + "" + ); + }); + it("should read iso date properly", () => { + const f = GetProjectFileWithOneField("DateAvailable", "2015-03-05"); + expect(f.properties.getDateField("dateAvailable").asISODateString()).toBe( + "2015-03-05" + ); + }); + it("should read iso date with time and offset properly", () => { + const f = GetProjectFileWithOneField( + "DateAvailable", + "2016-01-01T00:00:00+02:00" + ); + // The original says it's at midnight in a timezone 2 hours ahead of UTC. + // In SayMore we don't want to deal with timezones, so we convert that to + // UTC, which is actually the previous day, drop the time, drop the time offset. + expect(f.properties.getDateField("dateAvailable").asISODateString()).toBe( + "2015-12-31" + ); + }); +}); + +function GetProjectFileWithOneField( + tag: string, + content: string +): ProjectMetadataFile { + fs.writeFileSync( + Path.join(projectDirectory, projectName + ".sprj"), + ` + <${tag}>${content}` + ); + return new ProjectMetadataFile(projectDirectory); +} diff --git a/app/model/field/Field.ts b/app/model/field/Field.ts index fc90f92b..bed68150 100644 --- a/app/model/field/Field.ts +++ b/app/model/field/Field.ts @@ -66,7 +66,8 @@ export class Field { public readonly visibility: FieldVisibility; public persist: boolean; public readonly cssClass: string; - @observable public textHolder = new TextHolder(); + @observable + public textHolder = new TextHolder(); public choices: string[]; public definition: FieldDefinition; public contributorsArray: Contribution[]; //review @@ -145,11 +146,17 @@ export class Field { // //imdiIsClosedVocabulary?: boolean; // isCustom: false // }; - assert.ok( - this.key.toLowerCase().indexOf("date") === -1 || - this.type === FieldType.Date, - "SHOULDN'T " + key + " BE A DATE?" - ); + //assert.ok( + // this.key.toLowerCase().indexOf("date") === -1 || + // this.type === FieldType.Date, + // "SHOULDN'T " + key + " BE A DATE?" + //); + if ( + this.key.toLowerCase().indexOf("date") > -1 && + this.type !== FieldType.Date + ) { + console.error(key + " should be a date?"); + } } get text(): string { @@ -165,21 +172,27 @@ export class Field { this.text = s; } - public asDate(): Date { - return new Date(Date.parse(this.text)); - } + // public asDate(): Date { + // const x = new Date("2015-03-25Z"); + // const y = x.getUTCDate(); + // const z = this.text.indexOf("Z") > -1 ? this.text : this.text + "Z"; + // return new Date(this.text); + // } public asISODateString(): string { - if (moment(this.text).isValid()) { - return this.asDate().toISOString(); - } - return ""; + // our rule is that we always keep strings in "YYYY-MM-DD" format, and it's always UTC + return this.text; } public asLocaleDateString(): string { - if (moment(this.text).isValid()) { - return this.asDate().toLocaleDateString(); - } - return ""; + // if (moment(this.text).isValid()) { + // return this.asDate().toLocaleDateString(); + // } + // return ""; + + // maybe someday. But at the moment, javascript's date stuff is so eager to get into timezones + // that it's introducing buts. So for now let' keep it simple by just sticking to storing dates only as + // "YYYY-MM-DD" format string, and always UTC + return this.text; } public typeAndValueForXml(): [string, string] { switch (this.type) { diff --git a/app/model/field/FieldSet.ts b/app/model/field/FieldSet.ts index 1c55fcd1..b2568b4f 100644 --- a/app/model/field/FieldSet.ts +++ b/app/model/field/FieldSet.ts @@ -1,5 +1,5 @@ import { Dictionary } from "typescript-collections"; -import * as assert from "assert"; +import assert from "assert"; import { Field, FieldType, FieldDefinition, HasConsentField } from "./Field"; import { Contribution } from "../file/File"; import { Person } from "../Project/Person/Person"; @@ -47,11 +47,11 @@ export class FieldSet extends Dictionary { public addHasConsentProperty(person: Person) { this.setValue("hasConsent", new HasConsentField(person)); } - public manditoryTextProperty(key: string, value: string) { - if (!this.containsKey(key)) { - this.setValue(key, new Field(key, FieldType.Text, value)); - } - } + // public manditoryTextProperty(key: string, value: string) { + // if (!this.containsKey(key)) { + // this.setValue(key, new Field(key, FieldType.Text, value)); + // } + // } // public manditoryField(field: Field) { // if (this.containsKey(field.key)) { // const existing = this.getValue(field.key); diff --git a/app/model/file/File.ts b/app/model/file/File.ts index cb52d493..76054836 100644 --- a/app/model/file/File.ts +++ b/app/model/file/File.ts @@ -3,22 +3,26 @@ import * as fs from "fs"; import * as Path from "path"; const filesize = require("filesize"); import * as mobx from "mobx"; -import * as assert from "assert"; +import assert from "assert"; const camelcase = require("camelcase"); const imagesize = require("image-size"); import { Field, FieldType, FieldDefinition } from "../field/Field"; import { FieldSet } from "../field/FieldSet"; import * as xmlbuilder from "xmlbuilder"; import { locate } from "../../crossPlatformUtilities"; -const moment = require("moment"); +import moment from "moment"; const titleCase = require("title-case"); export class Contribution { //review this @mobx.observable - @mobx.observable public name: string; - @mobx.observable public role: string; - @mobx.observable public date: string; - @mobx.observable public comments: string; + @mobx.observable + public name: string; + @mobx.observable + public role: string; + @mobx.observable + public date: string; + @mobx.observable + public comments: string; } export abstract class File { @@ -28,7 +32,8 @@ export abstract class File { // In the case of folder objects (project, session, people) this will just be the metadata file, // and so describedFilePath === metadataPath. // In all other cases (mp3, jpeg, elan, txt), this will be the file we are storing metadata about. - @mobx.observable public describedFilePath: string; + @mobx.observable + public describedFilePath: string; // This file can be *just* metadata for a folder, in which case it has the fileExtensionForFolderMetadata. // But it can also be paired with a file in the folder, such as an image, sound, video, elan file, etc., @@ -40,9 +45,11 @@ export abstract class File { private fileExtensionForMetadata: string; public canDelete: boolean; - @mobx.observable public properties = new FieldSet(); + @mobx.observable + public properties = new FieldSet(); - @mobx.observable public contributions = new Array(); + @mobx.observable + public contributions = new Array(); get type(): string { const x = this.properties.getValue("type") as Field; @@ -56,12 +63,35 @@ export abstract class File { } } protected addDatePropertyFromString(key: string, dateString: string) { - // get a little paranoid with the date format - assert.ok(moment(dateString).isValid()); //todo: handle bad data - const date = new Date(Date.parse(dateString)); - this.checkType(key, date); - const dateWeTrust = date.toISOString(); - this.properties.setValue(key, new Field(key, FieldType.Date, dateWeTrust)); + // Note: I am finding it rather hard to not mess up dates, because in javascript + // everything (including moment) wants to over-think things and convert dates + // to one's local timezone. You get bugs like having the file say 2015-3-21, but + // then saving as 2015-3-20 because you're running this in America. Ugghhh. + // It's just too easy to mess up. So what I'm trying for now + // is to confine anything that could mess with the date to 2 places: here, + // at import time where we have to be permissive, and when displaying in the UI. + // Other than those two places, the rule is that all strings are YYYY-MM-DD in UTC. + + //assert.ok(moment(dateString).isValid()); //todo: handle bad data + + this.properties.setValue( + key, + new Field( + key, + FieldType.Date, + this.normalizeIncomingDateString(dateString) + ) + ); + } + protected normalizeIncomingDateString(dateString: string): string { + if (!dateString || dateString.trim().length === 0) { + return ""; + } + const date = moment(dateString); + const ISO_YEAR_MONTH_DATE_DASHES_FORMAT = "YYYY-MM-DD"; + // if there is time info, throw that away. + const standardizedDate = date.format(ISO_YEAR_MONTH_DATE_DASHES_FORMAT); + return standardizedDate; } protected addDateProperty(key: string, date: Date) { this.checkType(key, date); @@ -247,27 +277,34 @@ export abstract class File { ? key : camelcase(key); + // ---- DATES -- + if (key.toLowerCase().indexOf("date") > -1) { + const normalizedDateString = this.normalizeIncomingDateString(textValue); + if (this.properties.containsKey(fixedKey)) { + const existingDateField = this.properties.getValueOrThrow(fixedKey); + existingDateField.setValueFromString(normalizedDateString); + //console.log("11111" + key); + } else { + this.addDatePropertyFromString(fixedKey, normalizedDateString); + } + } + + // --- Text ---- // if it's already defined, let the existing field parse this into whatever structure (e.g. date) - if (this.properties.containsKey(fixedKey)) { + else if (this.properties.containsKey(fixedKey)) { const v = this.properties.getValueOrThrow(fixedKey); v.setValueFromString(textValue); //console.log("11111" + key); } else { - // Note: at least as of SayMore Windows 3.1, its files will have dates with the type "string" - // So we work around that by looking at the name of the key, to see if it contains the word "date" - if (key.toLowerCase().indexOf("date") > -1) { - this.addDatePropertyFromString(fixedKey, textValue); - } else { - //console.log("extra" + fixedKey + "=" + value); - // otherwise treat it as a string - this.addTextProperty( - fixedKey, - textValue, - true, - isCustom, - true /*showOnAutoForm*/ - ); - } + //console.log("extra" + fixedKey + "=" + value); + // otherwise treat it as a string + this.addTextProperty( + fixedKey, + textValue, + true, + isCustom, + true /*showOnAutoForm*/ + ); } } @@ -449,19 +486,20 @@ export abstract class File { ): xmlbuilder.XMLElementOrXMLNode { const ISO_YEAR_MONTH_DATE_DASHES_FORMAT = "YYYY-MM-DD"; if (dateString) { - if (moment(dateString).isValid()) { - const d = moment(dateString); - return builder - .element( - key, - // As of SayMore Windows 3.1.4, it can't handle a type "date"; it can only read and write a "string", - // so instead of the more reasonable { type: "date" }, we are using this - { type: "string" }, - d.format(ISO_YEAR_MONTH_DATE_DASHES_FORMAT) - ) - .up(); - } + // if (moment(dateString).isValid()) { + // const d = moment(dateString); + // return builder + // .element( + // key, + // // As of SayMore Windows 3.1.4, it can't handle a type "date"; it can only read and write a "string", + // // so instead of the more reasonable { type: "date" }, we are using this + // { type: "string" }, + // d.format(ISO_YEAR_MONTH_DATE_DASHES_FORMAT) + // ) + // .up(); + return builder.element(key, dateString).up(); } + return builder; // we didn't write anything } public save() { @@ -472,7 +510,7 @@ export abstract class File { //console.log(`skipping save of ${this.metadataFilePath}, not dirty`); return; } - console.log(`Saving ${this.metadataFilePath}`); + // console.log(`Saving ${this.metadataFilePath}`); const xml = this.getXml(); @@ -595,7 +633,7 @@ export abstract class File { } private clearDirty() { this.dirty = false; - console.log("dirty cleared " + this.metadataFilePath); + //console.log("dirty cleared " + this.metadataFilePath); } private changed() { diff --git a/tsconfig.json b/tsconfig.json index 0c6e5df8..591df0e2 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -18,7 +18,7 @@ "emitDecoratorMetadata": true, "allowSyntheticDefaultImports": true, "sourceMap": true, - + "esModuleInterop": true, "outDir": "dist" }, "files": ["app/index.tsx"],