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

Stock ownership availability rules #44

Merged
merged 19 commits into from
Jan 14, 2015

Conversation

lepistone
Copy link
Member

No description provided.

@lepistone lepistone force-pushed the stock_ownership_availability_rules branch from d1b4556 to d226477 Compare December 24, 2014 15:22
@@ -0,0 +1,7 @@
Stock Ownership Availability Rules
==================================

Copy link
Member

Choose a reason for hiding this comment

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

Can you please explain the purpose of the module?

Copy link
Member Author

Choose a reason for hiding this comment

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

done!

@coveralls
Copy link

Coverage Status

Coverage increased (+3.29%) when pulling d226477 on lepistone:stock_ownership_availability_rules into 4a5108e on OCA:8.0.

@coveralls
Copy link

Coverage Status

Coverage increased (+6.61%) when pulling 71ef53f on lepistone:stock_ownership_availability_rules into 4a5108e on OCA:8.0.

@lepistone lepistone changed the title WIP Stock ownership availability rules Stock ownership availability rules Jan 5, 2015
@coveralls
Copy link

Coverage Status

Coverage increased (+6.61%) when pulling 96674d9 on lepistone:stock_ownership_availability_rules into 4a5108e on OCA:8.0.

These tests are not necessary anymore, because the owner of the quant is
now required. A separate test to check that default is already in place.

Even before, "no owner" and "owned by myself" meant the same thing.

The tests were green.
@coveralls
Copy link

Coverage Status

Coverage increased (+6.61%) when pulling 441124e on lepistone:stock_ownership_availability_rules into 4a5108e on OCA:8.0.

# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
{'name': 'Stock Ownership Availability Rules',
'summary': 'XXX',
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove the entry or provide a meaningful summary?

I wrote this code when the owner of quants was not required, so I needed
to handle more cases. Now the logic can be simplified.

Moreover, If a location has a partner_id, I use that as a definition of
"own" stock.
@coveralls
Copy link

Coverage Status

Coverage increased (+6.33%) when pulling 74c62e9 on lepistone:stock_ownership_availability_rules into 4a5108e on OCA:8.0.

@lepistone
Copy link
Member Author

@gurneyalex you're right with your "side question": if a location has no partner, and no company (neither is required), what can I use as "own" stock? self.env.user.company_id.partner_id?

@gurneyalex
Copy link
Member

@lepistone: sounds good to me.

@gurneyalex
Copy link
Member

👍

@@ -0,0 +1,14 @@
from openerp import models
Copy link
Member

Choose a reason for hiding this comment

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

Missing header

@yvaucher
Copy link
Member

Apart from missing license header LGTM 👍

@lepistone
Copy link
Member Author

@yvaucher I added the missing headers. Feel free to merge 😸

@coveralls
Copy link

Coverage Status

Coverage increased (+16.88%) when pulling 6c64731 on lepistone:stock_ownership_availability_rules into 4a5108e on OCA:8.0.

@yvaucher
Copy link
Member

Thanks

yvaucher added a commit that referenced this pull request Jan 14, 2015
@yvaucher yvaucher merged commit b30ef17 into OCA:8.0 Jan 14, 2015
@lepistone lepistone deleted the stock_ownership_availability_rules branch January 20, 2015 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants