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

As a super admin, I should see a field I can use to link enterprise with my invoicing software #12942

Open
RachL opened this issue Oct 23, 2024 · 11 comments · May be fixed by #12980
Open

As a super admin, I should see a field I can use to link enterprise with my invoicing software #12942

RachL opened this issue Oct 23, 2024 · 11 comments · May be fixed by #12980
Assignees

Comments

@RachL
Copy link
Contributor

RachL commented Oct 23, 2024

⚠️ when working on this use #12942 Invoicing ID clockify code

Context : an OFN instance often needs to invoice one entity for several hubs. It's not simple to make accounting software holding that info.

This issue gives the ability to instance manager to store the ID their accounting software uses to invoice enterprise on the OFN enterprise profile.

Description

- As a: super admin
- On page: /admin/enterprises/XXXX/edit and /admin/reports/revenues_by_hub
- I want to be able to do:

As a super admin, I want to see a new field where I can put an ID. This ID can be a short combination of text (lower or upper caps)+number. What's important is that spaces are not allowed. E.g. : CU2410-00190.

Any other white-space characters too (eg tabs), using this handy tip.

Several enterprise can have the same ID.

This ID is only visible by super admins: enterprise owner / managers do not see them.

This ID should be used in the report that is only available for super admin called /admin/reports/revenues_by_hub

That means that super admin can use it in the report, the enterprises table in the database or the V0 API of this report.

Enterprise profile :

image

what's this? text proposal:

You can use this field to add the ID of your invoicing software, making it easier to calculate turnover per entity you are invoicing.

Report columns:

image

Acceptance Criteria & Tests

  1. Check you can add an ID on an enterprise profile
  2. This ID can be found on the revenue by hub report
  3. This ID is available on v0 API / report section, as well as the enterprise database
  4. Super admin can change the ID as many times as they want
  5. Super admin can't add spaces to this ID
  6. Several enterprises can have the same ID
  7. Enterprise owner / managers don't see this field
@github-project-automation github-project-automation bot moved this to All the things 💤 in OFN Delivery board Oct 23, 2024
@RachL RachL changed the title As a super admin, I should see a field I can use to link enterprise with my invoices system As a super admin, I should see a field I can use to link enterprise with my invoice software Oct 23, 2024
@RachL RachL changed the title As a super admin, I should see a field I can use to link enterprise with my invoice software As a super admin, I should see a field I can use to link enterprise with my invoicing software Oct 23, 2024
@BethanOFN
Copy link

Can invoicing id field also be added to the enterprises table in the database, for access via sql query?

@RachL
Copy link
Contributor Author

RachL commented Oct 24, 2024

yes!

@pacodelaluna
Copy link

@RachL I can take a look at this issue, when it is ready for development.

@dacook
Copy link
Member

dacook commented Oct 30, 2024

This new field is only visible to certain permission level (super admin), so I suggest we should display it separately from other 'normal' fields.
The clearest way to do this would be to add it to a new LHS tab (something like "Admin settings"), which only appears for super admins.
But there are already so many tabs, so maybe we don't want to clutter the screen so much! So another option would be to add a new section in the Primary Details tab (with red text and grey divider line). That should be quick and easy. What do you think Rachel?

As for the format validation, I assume this is to avoid input errors? I think that's a good idea, and would avoid being too restrictive, in case there is a need for special characters.
So the suggestion of limiting spaces (and allowing everything else) seems most sensible 👍. I would avoid any other white-space characters too (eg tabs), using this handy tip.

@RachL
Copy link
Contributor Author

RachL commented Oct 31, 2024

This new field is only visible to certain permission level (super admin), so I suggest we should display it separately from other 'normal' fields.
[...] So another option would be to add a new section in the Primary Details tab (with red text and grey divider line). That should be quick and easy. What do you think Rachel?

@dacook why not, I don't have a strong opinion. Note that currently this tab already displays info for super admin only (the radio buttons for "sells" and "visible in search ?") are shown only to super admin.

As for the format validation, I assume this is to avoid input errors?

yes I will rephrase to say that we want especially to not allow spaces.

@RachL RachL removed the blocked label Oct 31, 2024
@dacook
Copy link
Member

dacook commented Nov 4, 2024

Note that currently this tab already displays info for super admin only (the radio buttons for "sells" and "visible in search ?") are shown only to super admin.

I didn't realise that! Actually I just checked, and I think it's only the "Sells" field that is limited to super admin.

In this case, I suggest creating a new section at the bottom of the Primary Details tab, named something like "Admin Only". We can move "Sells", and this new "Client ID" under it. If you agree, I think we are ready for development!

(As a side note, it's a bit confusing that we don't have clearer nomenclature to differentiate between an Enterprise Admin and Instance Admin...)

@pacodelaluna
Copy link

@dacook If you want, I can go on with the development. Your inputs are pretty clear.
Maybe the last thing that we need to agree on is the name of this new field, client_id is good, but I was thinking it could be mismatched for a foreign key, no? I would suggest external_billing_uuid, but it may not be consistent with other namings, it is just an idea, better to be clear about it now.

About the steps to proceed:

  1. Add the field on the table in a new migration
  2. Add a validation on enterprise model as you precised it (Everything but spaces), with Rspec tests
  3. Place the field on the admin form for enterprises as you mentioned it (The renaming of Sells tab to Admin Only, with the new field)
  4. Add the field on the RevenueByHub report, with Rspec tests

Please tell me what you think about it, it is just a proposition. Or anyway, tell me how I can help you on this topic.

@dacook
Copy link
Member

dacook commented Nov 6, 2024

Thanks François.
Regarding the field name, I'm not sure of the best approach. I see what you mean about it looking like a foreign key.
But I think uuid is not quite correct: generally UUIDs are very large, designed to avoid conflicts, but this field is intended to hold any kind of IDs from any system.
So I think adding external is enough to indicate it's from an external system. How about external_billing_id?

Regarding all the other steps, yes that all looks good to me.
I would just add: it would be good to include the new client/billing ID in a system (browser) spec also, just confirming that the field can be saved. I think it could be added to the test "editing an existing enterprise".

@dacook

This comment was marked as resolved.

@RachL

This comment was marked as resolved.

@dacook

This comment was marked as resolved.

@pacodelaluna pacodelaluna linked a pull request Nov 16, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: All the things 💤
Development

Successfully merging a pull request may close this issue.

4 participants