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: Timezone set-up in MySettings was not honored when displaying date fields in the report #18438

Merged

Conversation

yrudman
Copy link
Contributor

@yrudman yrudman commented Feb 6, 2019

Issue:
Timezone for Display Settings set up in MySettings was not passed to column formatting when generating report.

When Timezone set-up as
screen shot 2019-02-06 at 1 49 32 pm

BEFORE:
before

AFTER:
after

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1599849

@miq-bot add-label bug, reporting

@yrudman yrudman force-pushed the apply-my-timezone-for-report-generation branch from 7eb2ec7 to 554bd0c Compare February 6, 2019 18:24
@yrudman yrudman changed the title [WIP] Fixed: Timezon set-up in MySettings was not honored when displaying date fields n the report Fixed: Timezon set-up in MySettings was not honored when displaying date fields in the report Feb 6, 2019
@yrudman
Copy link
Contributor Author

yrudman commented Feb 6, 2019

@yrudman
Copy link
Contributor Author

yrudman commented Feb 6, 2019

@miq-bot assign @gtanzillo

@yrudman yrudman force-pushed the apply-my-timezone-for-report-generation branch from 554bd0c to 0cc0ee3 Compare February 7, 2019 12:53
@jrafanie
Copy link
Member

jrafanie commented Feb 7, 2019

@yrudman can you add a test for this, we have one that's stubbing UTC that you can copy and change the timezone and add a check for the right timezone in the html rows..

see miq_report_spec.rb:

      it "returns expected html outputs with formatted values" do
        allow(User).to receive(:server_timezone).and_return("UTC")
        report.generate_table
        expect(report.build_html_rows).to match_array(@expected_html_rows)
      end

@yrudman yrudman force-pushed the apply-my-timezone-for-report-generation branch 2 times, most recently from d51f0c3 to ff39145 Compare February 8, 2019 19:08
@yrudman
Copy link
Contributor Author

yrudman commented Feb 8, 2019

@jrafanie test added, could you take another look at this PR

@jrafanie
Copy link
Member

A few typos in title, description, commit messages:

Timezon => Timezone
stabbing => stubbing
server_timezon => server_timezone
dfferent => different
Tme zone => Timezone
time zon => Timezone

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

Looks good, just a few typos mentioned above

@yrudman yrudman force-pushed the apply-my-timezone-for-report-generation branch from ff39145 to 0b7340f Compare February 11, 2019 19:41
@yrudman yrudman changed the title Fixed: Timezon set-up in MySettings was not honored when displaying date fields in the report Fixed: Timezone set-up in MySettings was not honored when displaying date fields in the report Feb 11, 2019
@yrudman yrudman closed this Feb 12, 2019
@yrudman yrudman reopened this Feb 12, 2019
@yrudman
Copy link
Contributor Author

yrudman commented Feb 12, 2019

@jrafanie spelling corrected

@jrafanie
Copy link
Member

There's still typos:

image

PR description:
Issue:
Tmezone

@yrudman yrudman force-pushed the apply-my-timezone-for-report-generation branch from 0b7340f to cc832d5 Compare February 12, 2019 18:43
@yrudman yrudman force-pushed the apply-my-timezone-for-report-generation branch from cc832d5 to 9e259ae Compare February 12, 2019 21:00
@miq-bot
Copy link
Member

miq-bot commented Feb 12, 2019

Checked commits yrudman/manageiq@8753ecf~...9e259ae with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 1 offense detected

spec/models/miq_report_spec.rb

@yrudman
Copy link
Contributor Author

yrudman commented Feb 12, 2019

@jrafanie I hope last typo was corrected

@jrafanie jrafanie assigned jrafanie and unassigned gtanzillo Feb 12, 2019
@jrafanie jrafanie added this to the Sprint 105 Ending Feb 18, 2019 milestone Feb 12, 2019
Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

Looks good, merging. Please apply backport labels is needed.

@jrafanie jrafanie merged commit 61e4c2a into ManageIQ:master Feb 12, 2019
@yrudman yrudman deleted the apply-my-timezone-for-report-generation branch February 13, 2019 13:23
@yrudman
Copy link
Contributor Author

yrudman commented Feb 13, 2019

@miq-bot add-label hammer/yes

simaishi pushed a commit that referenced this pull request Oct 17, 2019
…eneration

Fixed: Timezone set-up in MySettings was not honored when displaying date fields in the report
(cherry picked from commit 61e4c2a)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1761866
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit a9355a124cdb9ca98cec654c6b325c87d9c5edc7
Author: Joe Rafaniello <[email protected]>
Date:   Tue Feb 12 17:20:55 2019 -0500

    Merge pull request #18438 from yrudman/apply-my-timezone-for-report-generation
    
    Fixed: Timezone set-up in MySettings was not honored when displaying date fields in the report
    (cherry picked from commit 61e4c2adcdccc946cb39cac568ca5d2c25876a7b)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1761866

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

Successfully merging this pull request may close these issues.

5 participants