Allow branding to be preserved in download-for-edit (BL-13110)

DistFiles/localization/en/Bloom.xlf

@@ -4634,6 +4634,10 @@ Do you want to go ahead?</note>
         <note>ID: PublishTab.UploadCollisionDialog.AlreadyIn</note>
         <note>This is the header for the book that is in bloomlibrary.org already.</note>
       </trans-unit>
+      <trans-unit id="PublishTab.UploadCollisionDialog.ChangeBranding" translate="no">
+        <source xml:lang="en">The branding was "{0}" but is now "{1}". This may change logos and other material. Check this box if this is what you want.</source>
+        <note>ID: PublishTab.UploadCollisionDialog.ChangeBranding</note>
+      </trans-unit>
       <trans-unit id="PublishTab.UploadCollisionDialog.ChangeUploader" translate="no">
         <source xml:lang="en">Change the official uploader to {0}. (Bloom Library will hide part of your email address)</source>
         <note>ID: PublishTab.UploadCollisionDialog.ChangeUploader</note>

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";

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<IUploadCollisionDlgProp
     );
 
     const [doChangeUploader, setDoChangeUploader] = useState(false);
+    const [doChangeBranding, setDoChangeBranding] = useState(false);
 
     const kAskForHelpColor = "#D65649";
     const kDarkerSecondaryTextColor = "#555555";
@@ -137,6 +140,14 @@ export const UploadCollisionDlg: React.FunctionComponent<IUploadCollisionDlgProp
         "This is explanatory commentary on a radio button."
     );
 
+    const changeBrandingMessage = useL10n(
+        'The branding was "{0}" but is now "{1}". This may change logos and other material. Check this box if this is what you want.',
+        "PublishTab.UploadCollisionDialog.ChangeBranding",
+        "",
+        props.oldBranding,
+        props.newBranding
+    );
+
     const sameBookRadioLabel = useL10n(
         "Yes, I want to update this book",
         "PublishTab.UploadCollisionDialog.Radio.SameBook2",
@@ -247,6 +258,9 @@ export const UploadCollisionDlg: React.FunctionComponent<IUploadCollisionDlgProp
         </div>
     );
 
+    const needChangeBranding =
+        props.oldBranding && props.oldBranding !== props.newBranding;
+
     const differentBooksRadioCommentary = (): JSX.Element => (
         <div
             css={css`
@@ -286,6 +300,15 @@ export const UploadCollisionDlg: React.FunctionComponent<IUploadCollisionDlgProp
         closeDialog();
     }
 
+    const cssForCheckboxes = css`
+        margin-left: 37px;
+        .MuiFormControlLabel-root {
+            // The default 10px margin seems to me to visually break the connection between the checkboxes and
+            // their parent radio button.
+            padding-top: 2px !important;
+        }
+    `;
+
     return (
         <BloomDialog
             onCancel={() => {
@@ -482,15 +505,49 @@ export const UploadCollisionDlg: React.FunctionComponent<IUploadCollisionDlgProp
                             ariaLabel="Same book radio button"
                             commentaryChildren={sameBookRadioCommentary()}
                         />
+                        {canUpload && needChangeBranding && (
+                            <div css={cssForCheckboxes}>
+                                {/* 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. */}
+                                <BloomCheckbox
+                                    label={
+                                        <p>
+                                            <WarningIcon
+                                                color="warning"
+                                                css={css`
+                                                    // Aligns it with the text baseline
+                                                    position: relative;
+                                                    top: 2px;
+                                                    // Makes it a little smaller than using 'small' as the fontSize prop.
+                                                    // more in line with the mockup.
+                                                    font-size: 1em;
+                                                    margin-right: 5px;
+                                                `}
+                                            />
+                                            {changeBrandingMessage}
+                                        </p>
+                                    }
+                                    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);
+                                    }}
+                                ></BloomCheckbox>
+                            </div>
+                        )}
                         {canUpload &&
                             canBecomeUploader &&
                             props.uploader !== props.userEmail && (
-                                <div
-                                    css={css`
-                                        margin-left: 37px;
-                                        margin-top: -12px;
-                                    `}
-                                >
+                                <div css={cssForCheckboxes}>
                                     <BloomCheckbox
                                         label={changeTheUploader}
                                         alreadyLocalized={true}
@@ -538,7 +595,8 @@ export const UploadCollisionDlg: React.FunctionComponent<IUploadCollisionDlgProp
                     enabled={
                         // If we don't have permission to overwrite, we can only upload using a new ID
                         buttonState !== RadioState.Indeterminate &&
-                        (canUpload || buttonState === RadioState.Different)
+                        (canUpload || buttonState === RadioState.Different) &&
+                        (doChangeBranding || !needChangeBranding)
                     }
                     size="large"
                     onClick={() => {

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();
             }
 

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)
             }
         }
 
+        /// <summary>
+        /// 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.)
+        /// </summary>
+        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<string> 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;
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        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()
         {
@@ -847,6 +944,8 @@ public string CharactersForDigitsForPageNumbers
         private readonly Dictionary<string, string> ColorPalettes =
             new Dictionary<string, string>();
 
+        private string _brandingProjectKey;
+
         public string GetColorPaletteAsJson(string paletteTag)
         {
             var colorElementList = new List<string>();

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();
         }
 
         /// ------------------------------------------------------------------------------------

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;
+        }
+
         /// <summary>
         /// 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<string, bool> canUpload
                 return books;
             var remaining = books.ToList();
             var bookList = new List<dynamic>();
-            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

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);

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)