Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed loading the organization abbrevation from organization.json #951

Merged
merged 4 commits into from
Jun 26, 2020
Merged

Fixed loading the organization abbrevation from organization.json #951

merged 4 commits into from
Jun 26, 2020

Conversation

pphysch
Copy link
Contributor

@pphysch pphysch commented May 29, 2019

Description

Fixed the web portal always loading the organization's full name as its abbrevation, rather than the abbrevation provided by the admin during setup. Both strings are stored in the organization.json config file.

Motivation and Context

XDMoD allows the administrator to enter an abbrevation for their organization name, which may otherwise be lengthy ("CHPC" vs. "Center for High Performance Computing"). However, XDMoD has not been using the abbrevation for anything (except in /xdmod/classes/XDReportManager.php), perhaps because it has not been loaded correctly. This change fixes only the incorrect loading but does not change how the name/abbrev variables are used.

In future PRs it would be nice to change some instances of ORGANIZATION_NAME to ORGANIZATION_NAME_ABBREV in the web portal. For example, /xdmod/classes/DataWarehouse/Query/Jobs/Statistics/UtilizationStatistic.php includes 6 instances of the full name in its description paragraph, and the usage of the full name in the main statistics menu may lead to cropping on smaller monitors (especially when nesting queries). Most of these could be changed to the shorter abbrevation without losing any semantic clarity.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project as found in the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@jpwhite4
Copy link
Member

The ORGANIZATION_NAME_ABBREV is used to set the javascript variable CCR.xdmod.org_abbrev which in turn is used in the report generator to set the chart_drill_details. It is not obvious to me that this change will not have any side effects such as breaking the report generator for existing saved charts (that were saved in previous versions of the software). Have you done any testing / validation of the report generator with this change?

@pphysch
Copy link
Contributor Author

pphysch commented May 30, 2019

Thanks for your response.

I created a fresh XDMoD instance and a couple reports and verified they had empty chart_drill_details values in moddb.ReportCharts. With my name and abbrev as "Center for High Performance Computing" and "CHPC" respectively, I am able to see "Center for High Performance Computing" (i.e. ORGANIZATION_NAME_ABBREV) rendered as the subtitle in the report Preview, PDF, and Word Docs.

Then I applied this change live to the instance and the report Preview, PDF and Doc are now showing "CHPC" as the subtitle after the old reports are newly generated, as expected, with no visible errors.
before.pdf
after.pdf

As far as I understand, this is the only current usage of ORGANIZATION_NAME_ABBREV; the subtitle for reports that have empty chart_drill_details. It's not clear to me that the the abbreviation is even appropriate here over the full name. On both PDF and Word Document, the "subtitle" font size is much smaller than the word title and there is plenty of room for the full organization name, which is otherwise not present on the report unless included in one of the fields.

Therefore, I would further suggest that the existing usage of ORGANIZATION_NAME_ABBREV in the classes/XDReportManager.php be changed to ORGANIZATION_NAME, as both variables are already using identical string values (hence this PR). With this additional change there would be no change whatsoever to report content after a merge, so existing downloaded reports would have the same subtitle content as newly generated reports. This has the bonus of removing all current references to ORGANIZATION_NAME_ABBREV from the source so it may be easier to decide where to go with the organization abbreviation string. If this makes sense, I can add another commit to this PR and test it.

@jpwhite4 jpwhite4 added this to the 9.0.0 milestone Sep 19, 2019
@jpwhite4 jpwhite4 changed the base branch from xdmod8.5 to xdmod9.0 June 26, 2020 16:07
@jpwhite4 jpwhite4 merged commit 6d6ec85 into ubccr:xdmod9.0 Jun 26, 2020
@jtpalmer jtpalmer added bug Bugfixes Category:General General labels Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugfixes Category:General General
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants