Skip to content

Conversation

@flotho
Copy link
Member

@flotho flotho commented Nov 21, 2025

Also add active option

image

@flotho
Copy link
Member Author

flotho commented Nov 21, 2025

ping @DorianMAG @JulienMartinez

@legalsylvain
Copy link
Contributor

Hi.

  • adding active field : ✔️
  • adding kanban view : ✔️
  • adding partner / comercial partner field : I dont' see the interest, and I don't see it very clear. Could you elaborate ?
    thanks !

@flotho
Copy link
Member Author

flotho commented Nov 21, 2025

Hi @legalsylvain
Thanks for the code review

* adding partner / comercial partner field : I dont' see the interest, and I don't see it very clear. Could you elaborate ?
  thanks !

Most of time, we (Mind And Go / Datacup) are using sheets for a specific project, customer etc. We use sheets for computation but also for sharing simple quality process between us. Basically some checklist with colors.
Having a partner on sheets really helps to give context and find easily the document and prevent us to add partner name in the file name.

@legalsylvain
Copy link
Contributor

Hi @flotho. Thanks for your reply !

  • Could we imagine having a M2M fields regarding partners. it could be more generic. (and image can be computed as the image of the first partner).
  • I see the "commercial" partner quite specific. I mean, for the time being, this field is present only in sale / account module. for all the other modules. (crm / project / etc...) there is a simple partner_id.

@etobella : a point of view here ?

thanks !

@etobella
Copy link
Member

you are solving the issue by using partner, but wouldn't be cleaner to use res_model and res_id and access/generate them from the thread similar to mgmtsystem.nonconformity?

@flotho
Copy link
Member Author

flotho commented Nov 21, 2025

Hum, this leads us to a great path.
we're using commercial partner_id not for commercial purpose but to be able to group by customer company and not by the contact.
I'm totally fan of the M2M field @legalsylvain
@etobella , I'll have a look to your proposal.
Anyway, to ease the merging what could be acceptable in the first step ?
M2m on partner adn without commercial_partner_id ?
Regards

@legalsylvain
Copy link
Contributor

Anyway, to ease the merging what could be acceptable in the first step ?

I let @etobella and other main developpers answer on that topic. I didn't contributed on that great repo, and so don't have a clear PoV.

@chrisandrewmann
Copy link
Contributor

chrisandrewmann commented Nov 21, 2025

Thanks @flotho it's great to see and i'm 100% behind a kanban view.
My thoughts below, see the attached mockup image:

  • I think the icon should be static and possibly changeable - linked to an image field on the spreadsheet.
    Alternatively, a mechanism to generate a thumbnail preview could be used similar to Enterprise?
  • Cursor change and clickable image on left
  • Menu with 3 options (Open, Settings, Delete)
  • I'd support having the ability to set a partner_id, but not the commercial partner as I think that's possibly confusing
  • Instead of defaulting to commercial partner for grouping i'd suggest adding M2M tags, which could be displayed on the kanban card and also easily searchable / groupable
image

@flotho
Copy link
Member Author

flotho commented Nov 21, 2025

Thanks,
Here is my new proposal :
image

@legalsylvain legalsylvain marked this pull request as draft November 21, 2025 16:00
@legalsylvain
Copy link
Contributor

Hi @flotho. I put the PR in draft state, as it looks WIP, with red CI. Feel free to mark it as "Ready for review", once all is green and commit have been refactored.
thanks for this improvement !

@flotho flotho force-pushed the IMP-spreadsheet_oca branch from 52c210d to 831214d Compare November 21, 2025 16:17
@chrisandrewmann
Copy link
Contributor

chrisandrewmann commented Nov 21, 2025

@flotho looks really good, I like the new proposal.

Couple of things:

  • Should add a Configuration > Tags menu to manage the tags
  • If the image is missing, there should be a default placeholder (suggested the font-awesome icon of a spreadsheet in my example) - Otherwise it's empty whitespace

@flotho
Copy link
Member Author

flotho commented Nov 21, 2025

@flotho looks really good, I like the new proposal.

Couple of things:

* Should add a Configuration > Tags menu to manage the tags

* If the image is missing, there should be a default placeholder (suggested the font-awesome icon of a spreadsheet in my example)

you're right.
Yet I won't do it no.
Is it blocking for merging ?

@flotho flotho marked this pull request as ready for review November 21, 2025 16:19
@chrisandrewmann
Copy link
Contributor

@flotho looks really good, I like the new proposal.
Couple of things:

* Should add a Configuration > Tags menu to manage the tags

* If the image is missing, there should be a default placeholder (suggested the font-awesome icon of a spreadsheet in my example)

you're right. Yet I won't do it no. Is it blocking for merging ?

I'll wait for other comments @legalsylvain @etobella ?
Personally I think we should fix these 2 points for usability.

image

@flotho
Copy link
Member Author

flotho commented Nov 22, 2025

ping @legalsylvain @etobella @chrisandrewmann , ready for review

@chrisandrewmann
Copy link
Contributor

@flotho thanks, changes look good to me.
Quickly tested on runbot.
However CI tests failed due to missing alt tag on img. Should be simple fix, then we can review

@flotho
Copy link
Member Author

flotho commented Nov 22, 2025

@flotho thanks, changes look good to me. Quickly tested on runbot. However CI tests failed due to missing alt tag on img. Should be simple fix, then we can review

done !

Copy link
Contributor

@chrisandrewmann chrisandrewmann 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 to me, approved. Will wait for others

@flotho
Copy link
Member Author

flotho commented Nov 29, 2025

Hi @etobella @CarlosRoca13 @chrisandrewmann
Any chance to have a review on this small addition ?
Regards

Copy link
Member

@etobella etobella left a comment

Choose a reason for hiding this comment

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

I am in favor of the concept of kanban and tags, but I think it will be better to make the res_model, res_id strategy.

@flotho
Copy link
Member Author

flotho commented Dec 1, 2025

I am in favor of the concept of kanban and tags, but I think it will be better to make the res_model, res_id strategy.

Noted, thanks for the feedback

@pedrobaeza pedrobaeza added this to the 18.0 milestone Dec 1, 2025
@pedrobaeza
Copy link
Member

@flotho if you remove the partner_id part of this PR, this can be merged, and you can put that part in a extension on your side, or propose the generic way. You can also use a reference field.

@flotho flotho force-pushed the IMP-spreadsheet_oca branch from 50294c8 to 2013125 Compare December 9, 2025 22:36
@flotho
Copy link
Member Author

flotho commented Dec 10, 2025

Thanks @pedrobaeza , it's done ,
ping @etobella your review is welcomed

@pedrobaeza pedrobaeza changed the title [IMP] spreadsheet_oca: Add Kanban views [18.0][IMP] spreadsheet_oca: Add Kanban views Dec 10, 2025
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Please reduce commits into 2 (tags and kanban)

spreadsheet_oca.access_spreadsheet_spreadsheet_import,access_spreadsheet_spreadsheet_import,spreadsheet_oca.model_spreadsheet_spreadsheet_import,base.group_user,1,1,1,1
access_spreadsheet_import_mode,access_spreadsheet_oca_revision,model_spreadsheet_spreadsheet_import_mode,base.group_user,1,0,0,0
access_spreadsheet_select_row_number,access_spreadsheet_select_row_number,model_spreadsheet_select_row_number,base.group_user,1,1,1,1
access_spreadsheet_spreadsheet_tag,access_spreadsheet_spreadsheet_tag,model_spreadsheet_spreadsheet_tag,group_user,1,1,1,1
Copy link
Member

Choose a reason for hiding this comment

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

Only spreadsheet managers should change tags I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a matter of taste, IMO.

./src/OCA/manufacture/mrp_bom_tag/security/ir.model.access.csv:access_mrp_bom_tag_manager,Bill Of Material Tag Manager,model_mrp_bom_tag,mrp.group_mrp_user,1,1,1,1
./src/OCA/server-auth/vault/security/ir.model.access.csv:access_vault_tag,access_vault_tag,model_vault_tag,base.group_user,1,1,1,1
./src/odoo/addons/website_forum/security/ir.model.access.csv:access_forum_tag_user,forum.tag.user,model_forum_tag,base.group_user,1,1,1,1
./src/odoo/addons/repair/security/ir.model.access.csv:access_repair_tag_user,Repair Tags user,model_repair_tags,stock.group_stock_user,1,1,1,1

etc.

Copy link
Member

Choose a reason for hiding this comment

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

The examples you are putting, except the first one, it's because there's no manager role. Usually, all the "master data" are only accessible by managers of the area.

Copy link
Contributor

@legalsylvain legalsylvain Dec 10, 2025

Choose a reason for hiding this comment

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

The examples you are putting, except the first one, it's because there's no manager role.

I don't get it.
Regarding model_repair_tags for exemple. It is possible to manage tag with stock.group_stock_user group, not only stock.group_stock_manager. (group_stock_manager implies group_stock_user)
Or did I missed something ?

Copy link
Member

Choose a reason for hiding this comment

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

Please don't go off-topic. The topic here is that it's legit to not allow to modify tags to "simple" spreadsheet users, just the managers. That's it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible for you to speak correctly and on valid points?
That's not the first time, you're acting like an authoritarian leader. It's annoying to talk with you.
Thank you for questionning yourself on this point.

Copy link
Member

Choose a reason for hiding this comment

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

It's you the one going to off-topics, as I'm talking about spreadsheet tags, not repair tags. And I'm being totally correct. Please tell me in which moment I have not been correct. It's also annoying to talk to you, deviating conversations that are not targeted to you, but to @flotho, who is the author of the PR.

@pedrobaeza
Copy link
Member

Thanks for the renaming, @flotho

Can you please squash commits as suggested, and what do you think about the tag permissions?

* Add Kanban views
* Also add active option

[IMP] spreadsheet_oca:
* Simplify partner usage
* Add tags
* Improve kanban view

[IMP] spreadsheet_oca: Improve form view

[IMP] spreadhseet_oca: add tag menu

[IMP] spreadhseet_oca: add default image

IMP: Add tag on view

[REF] spreadsheet_oca: no more partner

[REF] spreadsheet_oca: no more partner

[REF] spreadsheet_oca: no more partner

[REF]rename file
[ACL]Ad specific rule for manager
@flotho flotho force-pushed the IMP-spreadsheet_oca branch from 89102ae to c7ea389 Compare December 11, 2025 09:13
@flotho
Copy link
Member Author

flotho commented Dec 11, 2025

done @pedrobaeza

@pedrobaeza
Copy link
Member

@flotho I was talking about 2 commits, as you are doing here 2 things: tags and kanban view. The commit message also lacks the first line explanations. And the tag question is still open.

@flotho
Copy link
Member Author

flotho commented Dec 11, 2025

oh damned I squashed too much ;-)
Do I need to undo ?
regarding ACL I've already changed the rules to have 2 one for basic user and another for manager

@pedrobaeza
Copy link
Member

Thank you for the change in the tags. About the commits, if it's not very difficult to you, it would be great to have both features split, but I don't want to press more. Just tell me and let's finish this.

<h1>
<field name="name" placeholder="Name" />
</h1>
<field name="badge_image" widget="image" class="oe_avatar" />
Copy link
Contributor

Choose a reason for hiding this comment

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

To enable "Archived" ribbon and ability to archive from form, add above:

<field name="active" invisible="1"/>
<widget name="web_ribbon" title="Archived" bg_color="text-bg-danger" invisible="active"/>

</group>
</sheet>
</form>
</field>
Copy link
Contributor

Choose a reason for hiding this comment

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

Noticed you added inherited "mail.thread", "mail.activity.mixin" to spreadsheet_spreadsheet.py but didn't enable chatter in the view. Think this would be a welcome simple change. Just add this line after the ending sheet tag:

<chatter/>

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

A minor remark. Otherwise, LGTM.

Thanks for this improvment.

spreadsheet_oca.access_spreadsheet_spreadsheet_import,access_spreadsheet_spreadsheet_import,spreadsheet_oca.model_spreadsheet_spreadsheet_import,base.group_user,1,1,1,1
access_spreadsheet_import_mode,access_spreadsheet_oca_revision,model_spreadsheet_spreadsheet_import_mode,base.group_user,1,0,0,0
access_spreadsheet_select_row_number,access_spreadsheet_select_row_number,model_spreadsheet_select_row_number,base.group_user,1,1,1,1
access_spreadsheet_spreadsheet_tag,access_spreadsheet_spreadsheet_tag,model_spreadsheet_spreadsheet_tag,spreadsheet_oca.group_user,1,0,1,0
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
access_spreadsheet_spreadsheet_tag,access_spreadsheet_spreadsheet_tag,model_spreadsheet_spreadsheet_tag,spreadsheet_oca.group_user,1,0,1,0
access_spreadsheet_spreadsheet_tag,access_spreadsheet_spreadsheet_tag,model_spreadsheet_spreadsheet_tag,spreadsheet_oca.group_user,1,1,1,0

At least. Otherwise, if a user create a tag, ans if he made a mistake in the name, he can not change it.

Copy link
Member

@pedrobaeza pedrobaeza Dec 11, 2025

Choose a reason for hiding this comment

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

Well, I still think that users should only use them (read them), not to modify them. If they are going to be able to create them, but not to remove them, that's worst.

Copy link
Contributor

Choose a reason for hiding this comment

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

What is your opinion here @flotho, and what do you need ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for sharing your PoV.
I'm personally used to install the module that prevent creation on the fly for a user because in the end it produce inconsistent data. Typically the use case @legalsylvain described : if a user create with a bad name
That's why I'm in favor of @pedrobaeza to prevent user to modify the tag. Keeping data consistent should be the responsibility of managers.

Copy link
Member

Choose a reason for hiding this comment

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

¿Then should it be this?:

Suggested change
access_spreadsheet_spreadsheet_tag,access_spreadsheet_spreadsheet_tag,model_spreadsheet_spreadsheet_tag,spreadsheet_oca.group_user,1,0,1,0
access_spreadsheet_spreadsheet_tag,access_spreadsheet_spreadsheet_tag,model_spreadsheet_spreadsheet_tag,spreadsheet_oca.group_user,1,0,0,0

Copy link
Contributor

@chrisandrewmann chrisandrewmann left a comment

Choose a reason for hiding this comment

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

Amazing, thanks for that. Looks good.

chrisandrewmann
chrisandrewmann approved these changes Dec 12, 2025
@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). 🤖

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Let's merge this although there's an extra commit for the last change.

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 18.0-ocabot-merge-pr-84-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit fe3f4b5 into OCA:18.0 Dec 16, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

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.

6 participants