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

[11.0] [MIG] base_import_security_group #58

Merged

Conversation

i-vyshnevska
Copy link
Member

@i-vyshnevska i-vyshnevska commented Apr 7, 2019

migrate module base_import_security_group

espo-tony and others added 7 commits May 20, 2019 02:35
This module makes importing data from CSV files optional for each user, allowing it only for those users belonging to a specific group.

[FIX] Solved flake8 error E305: expected 2 blank lines after class or function definition, found 1

[FIX] Renamed base_import_csv_optional in base_import_security_group

[FIX] Solved flake8 error E501: line too long;

[FIX] Removed unnecessary monkeypatch and extended Base.load method;

[FIX] Bugfixing in Base.load method;

[FIX] Code refactoring

[IMP] Tests created
[FIX] Updated travis postgresql version

[FIX] Updated phantom_js version in travis

[FIX] Configured travis to run tests for this module in isolation mode

[FIX] Configured travis to run tests for this module in isolation mode

[FIX] Added group to demo_user for test purposes

[FIX] Changed waiting condition on Phantom JS test

[FIX] Removed Travis configuration for isolated test environment

[FIX] Changed waiting condition in phantom_js test

[FIX] Changed target action in phantom_js test

[FIX] Changed target action in phantom_js test
@i-vyshnevska i-vyshnevska force-pushed the 11.0-mig-base_import_security_group branch from b41c4c9 to e6f6829 Compare May 19, 2019 23:35
Copy link
Member

@astirpe astirpe left a comment

Choose a reason for hiding this comment

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

Travis is red.

base_import_security_group/readme/DESCRIPTION.rst Outdated Show resolved Hide resolved
base_import_security_group/__manifest__.py Outdated Show resolved Hide resolved
@simahawk
Copy link
Contributor

simahawk commented Jul 4, 2019

@i-vyshnevska ping?

@i-vyshnevska i-vyshnevska changed the title 11.0 mig base import security group [11.0] [MIG] base_import_security_group Jul 17, 2019
@i-vyshnevska i-vyshnevska force-pushed the 11.0-mig-base_import_security_group branch from f922c3f to f46ec7d Compare July 17, 2019 07:41
@i-vyshnevska i-vyshnevska force-pushed the 11.0-mig-base_import_security_group branch from 4f8fe5b to ad9e8b0 Compare July 17, 2019 08:30
@i-vyshnevska
Copy link
Member Author

@astirpe thanks for review, updated

Copy link
Member

@astirpe astirpe left a comment

Choose a reason for hiding this comment

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

Code review OK.

Would be nice to fix some non-blocking warnings in travis, they are regarding javascript:

base_import_security_group/static/src/js/import.js:4: [W7903(javascript-lint), ] 'KanbanController' is assigned a value but never used. [Error/no-unused-vars]
base_import_security_group/static/src/js/import.js:6: [W7903(javascript-lint), ] 'ListController' is assigned a value but never used. [Error/no-unused-vars]
base_import_security_group/static/src/js/import.js:12: [W7903(javascript-lint), ] Expected indentation of 8 spaces but found 4. [Error/indent]
base_import_security_group/static/src/js/import.js:12: [W7903(javascript-lint), ] 'viewInfo' is defined but never used. [Error/no-unused-vars]
base_import_security_group/static/src/js/import.js:12: [W7903(javascript-lint), ] 'params' is defined but never used. [Error/no-unused-vars]
base_import_security_group/static/src/js/import.js:14: [W7903(javascript-lint), ] Expected indentation of 12 spaces but found 8. [Error/indent]
base_import_security_group/static/src/js/import.js:14: [W7903(javascript-lint), ] Missing semicolon. [Error/semi]
base_import_security_group/static/src/js/import.js:15: [W7903(javascript-lint), ] Expected indentation of 12 spaces but found 8. [Error/indent]
base_import_security_group/static/src/js/import.js:15: [W7903(javascript-lint), ] 'result' is assigned a value but never used. [Error/no-unused-vars]
base_import_security_group/static/src/js/import.js:16: [W7903(javascript-lint), ] Expected indentation of 12 spaces but found 8. [Error/indent]
base_import_security_group/static/src/js/import.js:18: [W7903(javascript-lint), ] Expected indentation of 12 spaces but found 8. [Error/indent]
base_import_security_group/static/src/js/import.js:18: [W7903(javascript-lint), ] 'result' is already declared in the upper scope. [Error/no-shadow]
base_import_security_group/static/src/js/import.js:18: [W7903(javascript-lint), ] Missing space before opening brace. [Error/space-before-blocks]
base_import_security_group/static/src/js/import.js:19: [W7903(javascript-lint), ] Expected indentation of 16 spaces but found 12. [Error/indent]
base_import_security_group/static/src/js/import.js:19: [W7903(javascript-lint), ] Missing semicolon. [Error/semi]
base_import_security_group/static/src/js/import.js:20: [W7903(javascript-lint), ] Expected indentation of 16 spaces but found 12. [Error/indent]
base_import_security_group/static/src/js/import.js:20: [W7903(javascript-lint), ] Missing space before opening brace. [Error/space-before-blocks]
base_import_security_group/static/src/js/import.js:21: [W7903(javascript-lint), ] Expected indentation of 20 spaces but found 16. [Error/indent]
base_import_security_group/static/src/js/import.js:21: [W7903(javascript-lint), ] Multiple spaces found before 'true'. [Error/no-multi-spaces]
base_import_security_group/static/src/js/import.js:22: [W7903(javascript-lint), ] Expected indentation of 16 spaces but found 12. [Error/indent]
base_import_security_group/static/src/js/import.js:23: [W7903(javascript-lint), ] Expected indentation of 16 spaces but found 12. [Error/indent]
base_import_security_group/static/src/js/import.js:30: [W7903(javascript-lint), ] Expected indentation of 8 spaces but found 4. [Error/indent]
base_import_security_group/static/src/js/import.js:31: [W7903(javascript-lint), ] Expected indentation of 12 spaces but found 8. [Error/indent]
base_import_security_group/static/src/js/import.js:32: [W7903(javascript-lint), ] Expected indentation of 12 spaces but found 8. [Error/indent]
base_import_security_group/static/src/js/import.js:37: [W7903(javascript-lint), ] Expected indentation of 8 spaces but found 4. [Error/indent]
base_import_security_group/static/src/js/import.js:38: [W7903(javascript-lint), ] Expected indentation of 12 spaces but found 8. [Error/indent]
base_import_security_group/static/src/js/import.js:39: [W7903(javascript-lint), ] Expected indentation of 12 spaces but found 8. [Error/indent]

Copy link

@hieulucky111 hieulucky111 left a comment

Choose a reason for hiding this comment

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

LGTM overall

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@pedrobaeza
Copy link
Member

/ocabot merge

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 11.0-ocabot-merge-pr-58-by-pedrobaeza-bump-no, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Sep 27, 2019
Signed-off-by pedrobaeza
@OCA-git-bot OCA-git-bot merged commit ad9e8b0 into OCA:11.0 Sep 27, 2019
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at dbe597b. Thanks a lot for contributing to OCA. ❤️

@kurogeek kurogeek mentioned this pull request May 5, 2020
12 tasks
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.

None yet

10 participants