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

Set the current userid when running a report #30

Merged

Conversation

imtayadeway
Copy link
Contributor

@imtayadeway imtayadeway commented Aug 21, 2017

With nothing specified it defaults to "system". Since we now enforce
checking the ownership of report results, the report results' userid
must be set to the current user instead.

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

@miq-bot add-label bug
@miq-bot assign @abellotti
/cc @gtanzillo @yrudman

UPDATE:

Just verified this.

Before:

GET http://localhost:3000/api/results/1r1

{
    "error": {
        "kind": "not_found",
        "message": "Couldn't find MiqReportResult with 'id'=1000000000001 [WHERE \"miq_report_results\".\"miq_group_id\" = 1000000000004]",
        "klass": "ActiveRecord::RecordNotFound"
    }
}

After

GET http://localhost:3000/api/results/1r2

{
    "href": "http://localhost:3000/api/results/1r2",
    "id": "1r2",
    "name": "VMs with Volume Free Space > 50% by Department",
    "miq_report_id": "1r1",
    "miq_task_id": "1r2",
    "userid": "tim",
    "report_source": "Requested by user",
    "db": "Vm",
    "created_on": "2017-08-21T23:53:22Z",
    "miq_group_id": "1r4",
    "result_set": []
}

With nothing specified it defaults to "system". Since we now enforce
checking the ownership of report results, the report results' userid
must be set to the current user instead.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1479296
@miq-bot
Copy link
Member

miq-bot commented Aug 21, 2017

Checked commit imtayadeway@5f73e46 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@gtanzillo
Copy link
Member

@imtayadeway I like this change 👍 much better that doing the revert. Just to be sure, were you able to recreated the reported issue and verify that this change addresses it?

@abellotti
Copy link
Member

👍 on this change.

@gtanzillo gtanzillo added this to the Sprint 67 Ending Aug 21, 2017 milestone Aug 22, 2017
@gtanzillo gtanzillo merged commit 2a38bb9 into ManageIQ:master Aug 22, 2017
@simaishi
Copy link
Contributor

Backported to Fine via ManageIQ/manageiq#15868

@imtayadeway imtayadeway deleted the bug/set-user-when-creating-report branch January 12, 2018 15:41
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