From cff47c88df2423cf99317eefe4187e6c9c03e3ad Mon Sep 17 00:00:00 2001 From: John Thomson Date: Wed, 6 Mar 2024 14:12:03 -0600 Subject: [PATCH] Allow branding to be preserved in download-for-edit (BL-13110) --- DistFiles/localization/en/Bloom.xlf | 4 + .../LibraryPublish/LibraryPublishSteps.tsx | 1 - .../LibraryPublish/uploadCollisionDlg.tsx | 72 ++++++++- src/BloomExe/Book/BookSelection.cs | 2 + src/BloomExe/Collection/CollectionSettings.cs | 143 +++++++++++++++++- src/BloomExe/Program.cs | 2 - .../BloomLibrary/BloomLibraryPublishModel.cs | 28 +++- src/BloomExe/Shell.cs | 5 - .../WebLibraryIntegration/BookDownload.cs | 5 + src/BloomExe/Workspace/WorkspaceView.cs | 45 +++++- .../web/controllers/CollectionSettingsApi.cs | 22 ++- 11 files changed, 298 insertions(+), 31 deletions(-) diff --git a/DistFiles/localization/en/Bloom.xlf b/DistFiles/localization/en/Bloom.xlf index dfe22c68b7ae..9f1e7829789f 100644 --- a/DistFiles/localization/en/Bloom.xlf +++ b/DistFiles/localization/en/Bloom.xlf @@ -4634,6 +4634,10 @@ Do you want to go ahead? ID: PublishTab.UploadCollisionDialog.AlreadyIn This is the header for the book that is in bloomlibrary.org already. + + The branding was "{0}" but is now "{1}". This may change logos and other material. Check this box if this is what you want. + ID: PublishTab.UploadCollisionDialog.ChangeBranding + Change the official uploader to {0}. (Bloom Library will hide part of your email address) ID: PublishTab.UploadCollisionDialog.ChangeUploader diff --git a/src/BloomBrowserUI/publish/LibraryPublish/LibraryPublishSteps.tsx b/src/BloomBrowserUI/publish/LibraryPublish/LibraryPublishSteps.tsx index 59fe86b55562..00dd9e85e29e 100644 --- a/src/BloomBrowserUI/publish/LibraryPublish/LibraryPublishSteps.tsx +++ b/src/BloomBrowserUI/publish/LibraryPublish/LibraryPublishSteps.tsx @@ -39,7 +39,6 @@ import { BloomSplitButton } from "../../react_components/bloomSplitButton"; import { ErrorBox, WaitBox } from "../../react_components/boxes"; import { IUploadCollisionDlgData, - IUploadCollisionDlgProps, showUploadCollisionDialog, UploadCollisionDlg } from "./uploadCollisionDlg"; diff --git a/src/BloomBrowserUI/publish/LibraryPublish/uploadCollisionDlg.tsx b/src/BloomBrowserUI/publish/LibraryPublish/uploadCollisionDlg.tsx index 099079a6fef9..467b71dae94c 100644 --- a/src/BloomBrowserUI/publish/LibraryPublish/uploadCollisionDlg.tsx +++ b/src/BloomBrowserUI/publish/LibraryPublish/uploadCollisionDlg.tsx @@ -51,6 +51,8 @@ export interface IUploadCollisionDlgData { existingBookUrl: string; existingThumbUrl?: string; uploader?: string; + oldBranding?: string; + newBranding?: string; onCancel?: () => void; dialogEnvironment?: IBloomDialogEnvironmentParams; permissions?: IPermissions; @@ -88,6 +90,7 @@ export const UploadCollisionDlg: React.FunctionComponent ); + const needChangeBranding = + props.oldBranding && props.oldBranding !== props.newBranding; + const differentBooksRadioCommentary = (): JSX.Element => (
{ @@ -482,15 +505,49 @@ export const UploadCollisionDlg: React.FunctionComponent + {canUpload && needChangeBranding && ( +
+ {/* The checkbox has an icon prop we could use instead of making it part of + the label, but the mockup has it wrapping as part of the first line of the + label, whereas our BloomCheckbox class puts it out to the left of all the lines + of label, and does something funny with positioning so that neither the icon nor + the text aligns with the text of the other checkbox when both are present. */} + + + {changeBrandingMessage} +

+ } + alreadyLocalized={true} + l10nKey="ignored" + checked={doChangeBranding} + onCheckChanged={() => { + // Enhance: it would probably be nice to select the appropriate radio button + // if it isn't already, but this is a rare special case (branding is rarely + // changed), we're trying to discourage doing it by accident, and it's not + // easy to actually take control of the radio button embedded in the + // RadioWithLabelAndCommentary from here. So for now, the user must do both.) + setDoChangeBranding(!doChangeBranding); + }} + > +
+ )} {canUpload && canBecomeUploader && props.uploader !== props.userEmail && ( -
+
{ diff --git a/src/BloomExe/Book/BookSelection.cs b/src/BloomExe/Book/BookSelection.cs index df5e280e26c3..08f1fac50a64 100644 --- a/src/BloomExe/Book/BookSelection.cs +++ b/src/BloomExe/Book/BookSelection.cs @@ -43,6 +43,8 @@ public void SelectBook(Book book, bool aboutToEdit = false) // BringUpToDate, typically only in unit tests. if (book != null && book.BookData != null && book.IsSaveable) { + // Before we bring it up to date, so it updates to the right branding + book.CollectionSettings.SetCurrentBook(book); book.EnsureUpToDate(); } diff --git a/src/BloomExe/Collection/CollectionSettings.cs b/src/BloomExe/Collection/CollectionSettings.cs index c2bea4176ca6..baadc913a7ba 100644 --- a/src/BloomExe/Collection/CollectionSettings.cs +++ b/src/BloomExe/Collection/CollectionSettings.cs @@ -10,11 +10,13 @@ using Bloom.Api; using Bloom.Book; using Bloom.MiscUI; +using Bloom.Publish.BloomLibrary; using Bloom.Publish.BloomPub; using Bloom.Utils; using Bloom.web.controllers; using DesktopAnalytics; using L10NSharp; +using Newtonsoft.Json.Linq; using SIL.Code; using SIL.Extensions; using SIL.IO; @@ -384,6 +386,29 @@ public static string CollectionIdFromCollectionFolder(string collectionFolder) } } + /// + /// Get the branding that the settings file specifies, without checking the subscription code + /// as we would do if creating a settings object from the settings file. (This is useful when + /// displaying the Branding dialog, to remind the user which branding they might want to find + /// a code for. We also use it to record the original branding for a book downloaded for editing, + /// since books on Bloom library keep their branding but not the code that normally allows it + /// to be used, though we make an exception for that one book.) + /// + public static string LoadBranding(string pathToCollectionFile) + { + try + { + var settingsContent = RobustFile.ReadAllText(pathToCollectionFile, Encoding.UTF8); + var xml = XElement.Parse(settingsContent); + return ReadString(xml, "BrandingProjectName", ""); + } + catch (Exception ex) + { + Bloom.Utils.MiscUtils.SuppressUnusedExceptionVarWarning(ex); + return ""; + } + } + /// ------------------------------------------------------------------------------------ public void Load() { @@ -662,7 +687,79 @@ internal IEnumerable GetAllLanguageTags() } // e.g. "ABC2020" or "Kyrgyzstan2020[English]" - public string BrandingProjectKey { get; set; } + public string BrandingProjectKey + { + get => _overrideBrandingForEditDownload ?? _brandingProjectKey; + set + { + _brandingProjectKey = value; + _overrideBrandingForEditDownload = null; + } + } + + private string _overrideBrandingForEditDownload; + + /// + /// Tell the collection which book we are currently working on. If + /// (a) this collection was made using "download for editing" on bloom library, and + /// (b) the current book is the one whose download created the collection, and + /// (c) as a result, both the book and the collection record some branding, but don't have the associated code, and + /// (d) the user hasn't changed the branding since the download that created the collection (which would establish + /// a new branding with a known code for all books in the collection including the original), + /// then we will allow the branding to be used for this book, even though we don't have the code. + /// + public void SetCurrentBook(Book.Book book) + { + if (book == null) + return; + // We allow a previous override to stand until some other book is selected. + // One reason is that CollectionModel.BringBookUpToDate changes the selection to null during the update, + // but we would like it to get updated to the right branding. + _overrideBrandingForEditDownload = null; + var downloadEditPath = Path.Combine( + Path.GetDirectoryName(book.FolderPath), + BloomLibraryPublishModel.kNameOfDownloadForEditFile + ); + if (!RobustFile.Exists(downloadEditPath)) + return; + try + { + var bookOfCollectionData = JObject.Parse(RobustFile.ReadAllText(downloadEditPath)); + //var databaseId = bookOfCollectionData["databaseId"]; + var instanceId = bookOfCollectionData["instanceId"]?.ToString(); + var bookFolder = bookOfCollectionData["bookFolder"]?.ToString(); + var brandingOfOriginalDownload = bookOfCollectionData["branding"]?.ToString(); + if ( + string.IsNullOrEmpty(brandingOfOriginalDownload) + || instanceId != book.ID + || bookFolder != book.FolderPath.Replace("\\", "/") + ) + return; // not validating as the one special book we can edit without the code (or it never had one) + // Now, are we actually in the situation where the collection specifies a branding but does not have + // a code for it? Initially, after a download-for-editing, the collection settingsfile (derived from the upload) + // may have a BrandingProjectName but never has a non-empty SubscriptionCode. This results in the + // CollectionSettings object having BrandingProjectKey set to Default (and InvalidBranding set to the + // one from the file). In that situation, we want to use that branding for the downloaded book. + // On the other hand, if the user later explicitly selected some branding, even Default, in the settings + // dialog, we don't want to use the downloaded one any more. When the user does that, the settings file gets saved, and we + // have a code if we need one, and there is no longer a discrepancy between the BrandingProjectName + // in the file and the BrandingProjectKey in the CollectionSettings object, and we no longer want + // an override, even for the original book. We might be able to determine this just from whether + // InvalidBranding is set, but I'm not sure that's always reliable. If the value stored in the file + // is different from the one in the CollectionSettings object, we want to override for this book. + // As a double-check, it should be the same branding we originally downloaded this book with. + var brandingInFile = LoadBranding(SettingsFilePath); + if ( + BrandingProjectKey != brandingInFile + && brandingOfOriginalDownload == brandingInFile + ) + _overrideBrandingForEditDownload = brandingOfOriginalDownload; + } + catch (Exception) + { + // If we can't process the file, just treat it as not the special book. + } + } public string GetBrandingFlavor() { @@ -680,17 +777,41 @@ out var flavor return folderName; } - public string SubscriptionCode { get; set; } + public string SubscriptionCode + { + get => _subscriptionCode; + set => _subscriptionCode = value; + } - public int OneTimeCheckVersionNumber { get; set; } + public int OneTimeCheckVersionNumber + { + get => _oneTimeCheckVersionNumber; + set => _oneTimeCheckVersionNumber = value; + } - public bool AllowNewBooks { get; set; } + public bool AllowNewBooks + { + get => _allowNewBooks; + set => _allowNewBooks = value; + } - public TalkingBookApi.AudioRecordingMode AudioRecordingMode { get; set; } + public TalkingBookApi.AudioRecordingMode AudioRecordingMode + { + get => _audioRecordingMode; + set => _audioRecordingMode = value; + } - public int AudioRecordingTrimEndMilliseconds { get; set; } + public int AudioRecordingTrimEndMilliseconds + { + get => _audioRecordingTrimEndMilliseconds; + set => _audioRecordingTrimEndMilliseconds = value; + } - public int BooksOnWebGoal { get; set; } + public int BooksOnWebGoal + { + get => _booksOnWebGoal; + set => _booksOnWebGoal = value; + } public BulkBloomPubPublishSettings BulkPublishBloomPubSettings = new BulkBloomPubPublishSettings @@ -847,6 +968,14 @@ public string CharactersForDigitsForPageNumbers private readonly Dictionary ColorPalettes = new Dictionary(); + private string _brandingProjectKey; + private string _subscriptionCode; + private int _oneTimeCheckVersionNumber; + private bool _allowNewBooks; + private TalkingBookApi.AudioRecordingMode _audioRecordingMode; + private int _audioRecordingTrimEndMilliseconds; + private int _booksOnWebGoal; + public string GetColorPaletteAsJson(string paletteTag) { var colorElementList = new List(); diff --git a/src/BloomExe/Program.cs b/src/BloomExe/Program.cs index 53b21d87ee7b..5bfa135dab43 100644 --- a/src/BloomExe/Program.cs +++ b/src/BloomExe/Program.cs @@ -1310,8 +1310,6 @@ private static void HandleProjectWindowActivated(object sender, EventArgs e) // Sometimes after closing the splash screen the project window // looses focus, so do this. _projectContext.ProjectWindow.Activate(); - - (_projectContext.ProjectWindow as Shell).CheckForInvalidBranding(); } /// ------------------------------------------------------------------------------------ diff --git a/src/BloomExe/Publish/BloomLibrary/BloomLibraryPublishModel.cs b/src/BloomExe/Publish/BloomLibrary/BloomLibraryPublishModel.cs index 57c5dc125a0d..21666d7e0f58 100644 --- a/src/BloomExe/Publish/BloomLibrary/BloomLibraryPublishModel.cs +++ b/src/BloomExe/Publish/BloomLibrary/BloomLibraryPublishModel.cs @@ -144,6 +144,19 @@ internal dynamic GetConflictingBookInfoFromServer(int index) return result; } + public static JObject GetDownloadForEditData(string pathToBookFolder) + { + var filePath = Path.Combine( + Path.GetDirectoryName(pathToBookFolder), + kNameOfDownloadForEditFile + ); + if (RobustFile.Exists(filePath)) + { + return JObject.Parse(RobustFile.ReadAllText(filePath)); + } + return null; + } + /// /// If we have multiple conflicting books, we want to sort them in a way that makes sense to the user. /// If we're in a collection that was made for editing one particular book, and this is the book, @@ -174,13 +187,9 @@ Func canUpload return books; var remaining = books.ToList(); var bookList = new List(); - var filePath = Path.Combine( - Path.GetDirectoryName(pathToBookFolder), - kNameOfDownloadForEditFile - ); - if (File.Exists(filePath)) + var bookOfCollectionData = GetDownloadForEditData(pathToBookFolder); + if (bookOfCollectionData != null) { - var bookOfCollectionData = JObject.Parse(RobustFile.ReadAllText(filePath)); var databaseId = bookOfCollectionData["databaseId"]; var instanceId = bookOfCollectionData["instanceId"]; var bookFolder = bookOfCollectionData["bookFolder"]?.ToString(); @@ -788,6 +797,11 @@ out string[] existingLanguages var updatedDate = updatedDateTime.ToString("d", CultureInfo.CurrentCulture); var existingThumbUrl = GetBloomLibraryThumbnailUrl(existingBookInfo); + // We could get it from the data about the download-for-edit book, but why limit this to those books? + //var bookDownloadForEditData = GetDownloadForEditData(Book.FolderPath); + //var oldBranding = bookDownloadForEditData?["branding"]?.ToString(); + var oldBranding = existingBookInfo.brandingProjectName?.ToString(); + // Must match IUploadCollisionDlgProps in uploadCollisionDlg.tsx. return new { @@ -803,6 +817,8 @@ out string[] existingLanguages existingUpdatedDate = updatedDate, existingBookUrl, existingThumbUrl, + newBranding = Book.BookInfo.BrandingProjectKey, + oldBranding, uploader = existingBookInfo.uploader.email, count = existingBookInfo.count, permissions = existingBookInfo.permissions diff --git a/src/BloomExe/Shell.cs b/src/BloomExe/Shell.cs index ad82624e8269..5e564ec49695 100644 --- a/src/BloomExe/Shell.cs +++ b/src/BloomExe/Shell.cs @@ -112,11 +112,6 @@ AudioRecording audioRecording SetWindowText(null); } - public void CheckForInvalidBranding() - { - _workspaceView.CheckForInvalidBranding(); - } - protected override void OnHandleCreated(EventArgs e) { base.OnHandleCreated(e); diff --git a/src/BloomExe/WebLibraryIntegration/BookDownload.cs b/src/BloomExe/WebLibraryIntegration/BookDownload.cs index ef206369d354..e9cd64ce6a9a 100644 --- a/src/BloomExe/WebLibraryIntegration/BookDownload.cs +++ b/src/BloomExe/WebLibraryIntegration/BookDownload.cs @@ -343,6 +343,11 @@ private void OnDoDownload(object sender, DoWorkEventArgs args) editData["databaseId"] = link.DatabaseId; editData["instanceId"] = id; editData["bookFolder"] = LastBookDownloadedPath.Replace("\\", "/"); + // We can't create an instance and read the branding, because load will wipe it out when it sees no code. + var branding = CollectionSettings.LoadBranding( + CollectionCreatedForLastDownload + ); + editData["branding"] = branding; RobustFile.WriteAllText( pathToForEditDataFile, Newtonsoft.Json.JsonConvert.SerializeObject(editData) diff --git a/src/BloomExe/Workspace/WorkspaceView.cs b/src/BloomExe/Workspace/WorkspaceView.cs index 9d52e82949db..82eb3246d01d 100644 --- a/src/BloomExe/Workspace/WorkspaceView.cs +++ b/src/BloomExe/Workspace/WorkspaceView.cs @@ -23,6 +23,7 @@ using Bloom.web.controllers; using L10NSharp; using Messir.Windows.Forms; +using NDesk.DBus; using Newtonsoft.Json; using SIL.IO; using SIL.PlatformUtilities; @@ -343,6 +344,30 @@ private void HandleBookSelectionChanged(object sender, BookSelectionChangedEvent var result = GetCurrentSelectedBookInfo(); // Important for at least the TeamCollectionBookStatusPanel and the CollectionsTabBookPanel. _webSocketServer.SendString("book-selection", "changed", result); + // If the collection specifies a branding but not a code to validate it, we want to + // notify the user and let them provide the code. The best time to do this is not obvious. + // One of the common cases for needing this dialog is after making a collection with + // "download for editing", which often has a branding but initially never has a code. + // But in that case very often we don't need to do it immediately, + // since that book is probably selected and (by a special exception) we allow it to use + // its original branding without a code. But if some other book gets selected, it won't + // use the branding without a code, and we want to notify the user about that. + // So we settled on triggering the dialog when the book selection changes (this method DOES + // get called at startup for any automatically-initially-selected book). The logic in + // CheckForInvalidBranding makes sure we don't bring up the dialog more than once or if + // it's not needed for the current book. + // So, if a collection has a branding but no code, and the special exception book is + // selected, we'll wait until the user selects a different book to bring up the dialog. + // If some other book is initially selected, we'll bring up the dialog immediately. + // It can cause some problems if we try to bring up the dialog in the middle of selecting + // a book, so we do it on idle. + Application.Idle += CheckForInvalidBrandingOnIdle; + } + + private void CheckForInvalidBrandingOnIdle(object sender, EventArgs e) + { + Application.Idle -= CheckForInvalidBrandingOnIdle; + CheckForInvalidBranding(); } string _tempBookInfoHtmlPath; @@ -944,12 +969,17 @@ public void OpenLegacySettingsDialog(string tab = null) BloomMessageBox.ShowInfo(MustBeAdminMessage); return DialogResult.Cancel; } + CollectionSettingsApi.SetUpLegacyBrandingForSettingsDialog( + _collectionSettings.InvalidBranding, + _collectionSettings.SubscriptionCode + ); using (var dlg = _settingsDialogFactory()) { _currentlyOpenSettingsDialog = dlg; dlg.SetDesiredTab(tab); var temp = dlg.ShowDialog(this); _currentlyOpenSettingsDialog = null; + CollectionSettingsApi.EndFixEnterpriseBranding(); return temp; } }); @@ -960,10 +990,23 @@ public void OpenLegacySettingsDialog(string tab = null) } } + private bool _haveCheckedForInvalidBranding; + public void CheckForInvalidBranding() { - if (_collectionSettings.InvalidBranding == null) + // We only do this once, only if we have recorded an invalid branding, only if we haven't established + // some other branding, and only if we've selected a book in the editable collection. (And it must not + // be the original download book, if any, for this collection...if it was that book, we'd already + // be simulating its original branding, and the Default branding test would fail.) + if ( + _haveCheckedForInvalidBranding + || _collectionSettings.InvalidBranding == null + || _collectionSettings.BrandingProjectKey != "Default" + || _bookSelection.CurrentSelection == null + || !_bookSelection.CurrentSelection.IsInEditableCollection + ) return; + _haveCheckedForInvalidBranding = true; // I'm not very happy with this, but the only place I could find to detect that we're opening a new project // is too soon to bring up a dialog; it comes up before the main window is fully initialized, which can // leave the main window in the wrong place. Waiting until idle gives a much better effect. diff --git a/src/BloomExe/web/controllers/CollectionSettingsApi.cs b/src/BloomExe/web/controllers/CollectionSettingsApi.cs index efe930c8b412..e0efbfb1c3df 100644 --- a/src/BloomExe/web/controllers/CollectionSettingsApi.cs +++ b/src/BloomExe/web/controllers/CollectionSettingsApi.cs @@ -459,8 +459,16 @@ private void HandleGetLanguageNames(ApiRequest request) // in BookSettingsDialog.tsx. x["language1Name"] = _bookSelection.CurrentSelection.CollectionSettings.Language1.Name; x["language2Name"] = _bookSelection.CurrentSelection.CollectionSettings.Language2.Name; - if (!String.IsNullOrEmpty(_bookSelection.CurrentSelection.CollectionSettings.Language3?.Name)) - x["language3Name"] = _bookSelection.CurrentSelection.CollectionSettings.Language3.Name; + if ( + !String.IsNullOrEmpty( + _bookSelection.CurrentSelection.CollectionSettings.Language3?.Name + ) + ) + x["language3Name"] = _bookSelection + .CurrentSelection + .CollectionSettings + .Language3 + .Name; request.ReplyWithJson(JsonConvert.SerializeObject(x)); } @@ -637,8 +645,18 @@ string subscriptionCode ) { FixEnterpriseSubscriptionCodeMode = true; + SetUpLegacyBrandingForSettingsDialog(invalidBranding, subscriptionCode); + } + + public static void SetUpLegacyBrandingForSettingsDialog( + string invalidBranding, + string subscriptionCode + ) + { if (SubscriptionCodeLooksIncomplete(subscriptionCode)) LegacyBrandingName = invalidBranding; // otherwise we won't show the legacy branding message, just bring up the dialog and show whatever's wrong. + else + LegacyBrandingName = ""; } public static void EndFixEnterpriseBranding()