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

Biz model additions #2931

Merged
merged 19 commits into from
Jan 9, 2023
Merged

Biz model additions #2931

merged 19 commits into from
Jan 9, 2023

Conversation

invisig0th
Copy link
Contributor

No description provided.

Copy link
Contributor

@therealsilence therealsilence left a comment

Choose a reason for hiding this comment

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

Few minor nits picked...

synapse/models/biz.py Outdated Show resolved Hide resolved
synapse/models/orgs.py Outdated Show resolved Hide resolved
synapse/models/orgs.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Base: 97.20% // Head: 97.09% // Decreases project coverage by -0.10% ⚠️

Coverage data is based on head (47ec81a) compared to base (c22a850).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2931      +/-   ##
==========================================
- Coverage   97.20%   97.09%   -0.11%     
==========================================
  Files         217      217              
  Lines       42773    42758      -15     
==========================================
- Hits        41578    41518      -60     
- Misses       1195     1240      +45     
Flag Coverage Δ
linux 97.09% <ø> (-0.01%) ⬇️
linux_replay ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
synapse/models/biz.py 100.00% <ø> (ø)
synapse/models/economic.py 100.00% <ø> (ø)
synapse/models/inet.py 99.53% <ø> (ø)
synapse/models/orgs.py 100.00% <ø> (ø)
synapse/models/risk.py 100.00% <ø> (ø)
synapse/tests/utils.py 94.46% <0.00%> (-1.89%) ⬇️
synapse/cortex.py 96.54% <0.00%> (-0.57%) ⬇️
synapse/lib/hiveauth.py 96.01% <0.00%> (-0.48%) ⬇️
synapse/lib/oauth.py 98.68% <0.00%> (-0.44%) ⬇️
synapse/lib/trigger.py 95.01% <0.00%> (-0.39%) ⬇️
... and 15 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

synapse/models/biz.py Outdated Show resolved Hide resolved
synapse/models/biz.py Outdated Show resolved Hide resolved
synapse/models/biz.py Show resolved Hide resolved
synapse/models/biz.py Show resolved Hide resolved
@invisig0th invisig0th changed the title WIP: Biz model additions Biz model additions Jan 9, 2023
mikemoritz
mikemoritz previously approved these changes Jan 9, 2023
synapse/models/economic.py Outdated Show resolved Hide resolved
Cisphyx
Cisphyx previously approved these changes Jan 9, 2023
mbvertex
mbvertex previously approved these changes Jan 9, 2023
@invisig0th invisig0th dismissed stale reviews from mbvertex, Cisphyx, and mikemoritz via eb5c2e0 January 9, 2023 17:48
}),
)),
('biz:listing', {}, (
Copy link
Contributor

Choose a reason for hiding this comment

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

Would a :loc property be useful here to track where the product/service is being offered (where relevant)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would require another round of discussions. Feel free to submit a ticket.

}),
)),
('biz:listing', {}, (
Copy link
Contributor

Choose a reason for hiding this comment

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

What a :url prop to track where the listed was being sold online?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, would require further discussions. Submit a ticket if you think it's valuable.

}),
)),
('biz:listing', {}, (
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a :desc prop to capture product/service descriptions provided by the seller?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another good idea for a ticket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe roll them up into one ticket for additional suggested fields. Model PR reviews are mostly for show stoppers / necessary clarifications rather than suggesting new fields. 👍

@invisig0th invisig0th merged commit c1c51c6 into master Jan 9, 2023
@invisig0th invisig0th deleted the visi-turkey-model branch January 9, 2023 18:57
@vEpiphyte vEpiphyte added this to the v2.11x.x milestone Jan 9, 2023
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.

7 participants