Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Migration Status string constants for i18n #476

Merged
merged 7 commits into from
Jul 11, 2018

Conversation

AparnaKarve
Copy link
Contributor

@AparnaKarve AparnaKarve commented Jul 9, 2018

@AparnaKarve
Copy link
Contributor Author

@fdupont-redhat It looks like the manageiq-content repo does not currently have a provision to extract strings for gettext.

I have captured all the Migration Status strings here for i18n that are visible in the UI.
Can you review these and let me know if I included them all?
Thanks!!

Copy link
Member

@AllenBW AllenBW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ ❤️ ❤️

wonderfully executed @AparnaKarve!!

@ghost
Copy link

ghost commented Jul 10, 2018

@AparnaKarve
It's probable that I will have more messages as I'm adding more states to the state machine. Should I also create PRs to add them?

@AparnaKarve
Copy link
Contributor Author

@fdupont-redhat Yes, please.
As you add more messages to the state machine, you can create a PR in this repo and add the new messages under V2V_MIGRATION_STATUS_MESSAGES

@AparnaKarve
Copy link
Contributor Author

AparnaKarve commented Jul 10, 2018

@priley86 Can you review the third commit?
I have replaced the Transformation word with Migration in most cases, except one -
'Acquire Transformation Host'

@priley86
Copy link
Member

priley86 commented Jul 10, 2018

this is much appreciated @AparnaKarve!

do you mind also updating the references in PlanRequestDetailListConstants.js:

export const ACTIVE_PLAN_FILTER_TYPES = [
  {
    id: 'vmName',
    title: __('VM Name'),
    placeholder: __('Filter by VM Name'),
    filterType: 'text'
  },
  {
    id: 'message',
    title: __('Status'),
    placeholder: __('Filter by Status'),
    filterType: 'select',
    filterValues: [
      { title: __('Pending'), id: 'Pending' },
      { title: __('Validating'), id: 'Validating' },
      { title: __('Pre-migration'), id: 'Pre-migration' },
      { title: __('Migrating'), id: 'Migrating' },
      {
        title: __('VM Transformations completed'),
        id: 'VM Transformations completed'
      },
      {
        title: __('VM Transformations failed'),
        id: 'VM Transformations failed'
      }
    ]
  }
];

These filter types should depend on those references now I think.

Otherwise LGTM.

@AparnaKarve AparnaKarve force-pushed the i18n_migration_status_constants branch from 5e4d543 to a9f7e72 Compare July 11, 2018 16:39
change sorting option from `Started` to `Elapsed Time`
@AparnaKarve AparnaKarve force-pushed the i18n_migration_status_constants branch from a9f7e72 to 0b8a6f8 Compare July 11, 2018 16:53
@AparnaKarve
Copy link
Contributor Author

@priley86 So, we don't need any changes in PlanRequestDetailListConstants.js.

Since we changed the Transformations word to Migrations in some cases, this has affected some areas where we actually check for the output'VM Transformations completed' or 'VM Transformations failed' -- hence I created constant values for those here 17512ba which I will be using for comparing with the API output.

@priley86
Copy link
Member

@AparnaKarve - i think we can just expose those keys as another constant...and reference them there for consistency. Let me know if this works for you...seems to test OK here.

priley86@f464506

@priley86
Copy link
Member

also - i think this now closes #255 😸

@AparnaKarve
Copy link
Contributor Author

@priley86 Thank you for 0a9d9a6

@priley86 / @AllenBW This is GTG, IMO.

Copy link
Member

@AllenBW AllenBW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the use of the keys 😻 Took me a sec to get it... buttttt this feels really solid, now we gotta remain vigilante, ensure they are USED

transformation_host_name: task.options && task.options.transformation_host_name,
delivered_on: new Date(task.options.delivered_on),
updated_on: new Date(task.updated_on),
completed: task.message === 'VM Transformations completed' || task.message === 'VM Transformations failed',
completed:
task.message === STATUS_MESSAGE_KEYS.VM_MIGRATIONS_COMPLETED ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love love love this <3

@AllenBW AllenBW merged commit 8646fc4 into ManageIQ:master Jul 11, 2018
@AllenBW AllenBW self-assigned this Jul 11, 2018
@AparnaKarve AparnaKarve deleted the i18n_migration_status_constants branch July 11, 2018 22:58
@bthurber
Copy link

simaishi pushed a commit that referenced this pull request Jul 31, 2018
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit f93006142f5faec92bff97970c4bf8989613dbe6
Author: Allen Wight <allen.b.wight@gmail.com>
Date:   Wed Jul 11 17:57:46 2018 -0400

    Merge pull request #476 from AparnaKarve/i18n_migration_status_constants
    
    Migration Status string constants for i18n
    (cherry picked from commit 8646fc4446716884458026d4f309d8b5d9aa033b)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1608752

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants