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

Update metrics #42

Merged
merged 10 commits into from
Jul 20, 2023
Merged

Conversation

KaterynaSloboda
Copy link
Contributor

@KaterynaSloboda KaterynaSloboda commented Jun 14, 2023

  • The total number of seats is replaced by the number of seats in each room separately in each layout function
  • Added counting of seats in LayoutFunctionCommon.LayoutStrategies class (used for LoungeLayout)
  • Added seat count for LoungeLayout function
  • Added another option for Calculation Mode in the Workplace Metrics function: "Calculated Headcount". In this mode, the number of employees is calculated programmatically

This change is Reviewable

@serenayl serenayl changed the title Update metrics for TMA Update metrics Jun 14, 2023
@serenayl serenayl self-requested a review June 14, 2023 19:18
Copy link
Contributor

@serenayl serenayl left a comment

Choose a reason for hiding this comment

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

Is there a way for "Fixed Sharing Ratio" to still yield the same numbers? For example, if I override headcount manually on one of the spaces, I would expect that calculating by "fixed sharing ratio" which is a multiplier on the number of desks should give me a different answer?

Otherwise this is looking really great, I have staged what you have but I may not release until this weekend. We are currently doing workshops and things so I don't want to break anything before then!

Reviewed 27 of 37 files at r1, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained, 2 unresolved discussions (waiting on @KaterynaSloboda)


WorkplaceStrategy/WorkplaceMetrics/hypar.json line 148 at r1 (raw file):

      }
    },
    "WorkpointCount": {

For users, can we use a space here? "Workpoint Count"


WorkplaceStrategy/WorkplaceMetrics/hypar.json line 158 at r1 (raw file):

      "paradigm": "edit",
      "schema": {
        "Count": {

Can this also be "Workpoint Count"?

Copy link
Contributor

@serenayl serenayl left a comment

Choose a reason for hiding this comment

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

Can we also have the workplace metrics ratio multiplier default to 1 instead of 1.5?

Reviewable status: 1 change requests, 0 of 1 approvals obtained, 2 unresolved discussions (waiting on @KaterynaSloboda)

Copy link
Contributor

@serenayl serenayl left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 change requests, 0 of 1 approvals obtained, 2 unresolved discussions (waiting on @KaterynaSloboda)


WorkplaceStrategy/WorkplaceMetrics/hypar.json line 148 at r1 (raw file):

Previously, serenayl (Serena Li) wrote…

For users, can we use a space here? "Workpoint Count"

Sorry I changed my mind, I want this to be "Workplace Metrics"


WorkplaceStrategy/WorkplaceMetrics/hypar.json line 158 at r1 (raw file):

Previously, serenayl (Serena Li) wrote…

Can this also be "Workpoint Count"?

Actually, this will have a new name. Our awesome product person also almost named Kateryna will be commenting on the Notion doc with what we want all the language to be!

@KaterynaSloboda KaterynaSloboda deleted the update-metrics-for-tma branch June 27, 2023 08:59
@KaterynaSloboda KaterynaSloboda restored the update-metrics-for-tma branch June 27, 2023 09:03
Copy link
Contributor

@serenayl serenayl left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained, 3 unresolved discussions (waiting on @KaterynaSloboda)


WorkplaceStrategy/WorkplaceMetrics/hypar.json line 167 at r1 (raw file):

    {
      "unit_type": "area",
      "name": "Total Usable Floor Area",

I'm sorry, I think I may have given the wrong direction in the Notion task. Can we revert the names on these (because they break widgets in the template), and then use messages to give them the name we want?

Copy link
Contributor

@serenayl serenayl left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 37 files at r1, 9 of 16 files at r2, 17 of 17 files at r3, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained, 3 unresolved discussions (waiting on @KaterynaSloboda)


LayoutFunctions/LoungeLayout/src/LoungeLayout.cs line 59 at r3 (raw file):

            ["https://hypar.io/user-static/b1e7fd86-3124-4f9d-9e0d-650f453df774.glb"] = 2,
            ["https://hypar.io/user-static/cdc18dac-396b-474d-a20b-2b1670a22a1a.glb"] = 2,
        };

This does not appear to be used, is this for a future PR?

Code quote:

        private static Dictionary<string, int> _countableSeaters = new Dictionary<string, int>()
        {
            ["https://hypar.io/user-static/28ec56cc-3ae5-40df-a87f-9f116c1f9c62.glb"] = 1,
            ["https://hypar.io/user-static/2dcb44e3-71e5-4d50-93ba-c3abb259186f.glb"] = 1,
            ["https://hypar.io/user-static/31c06210-c409-4086-a496-1ec3093deddc.glb"] = 1,
            ["https://hypar.io/user-static/32534d22-4622-44ac-89d2-395f755ee4d9.glb"] = 1,
            ["https://hypar.io/user-static/47ba6d86-89ca-47b0-81e1-051919514ebd.glb"] = 1,
            ["https://hypar.io/user-static/48d2887c-543e-4b57-bc58-2cf4a132d5e7.glb"] = 1,
            ["https://hypar.io/user-static/75ccd3c4-4817-422f-86c5-f522ada7de37.glb"] = 1,
            ["https://hypar.io/user-static/8f5418c7-ad8b-474d-aa90-322f989f018c.glb"] = 1,
            ["https://hypar.io/user-static/93a62034-8f46-4312-a4be-bc043fe33206.glb"] = 1,
            ["https://hypar.io/user-static/a5c4260c-fd4e-4977-a4d0-a5f18ce283a2.glb"] = 1,
            ["https://hypar.io/user-static/b44b8d75-fc93-44e3-bca3-9c15502ffa25.glb"] = 1,
            ["https://hypar.io/user-static/c84d0092-962f-497e-ba1a-d5b25b61d1f4.glb"] = 1,
            ["https://hypar.io/user-static/d9dba349-e9e0-4d4e-a9b9-6e4652291f0c.glb"] = 1,
            ["https://hypar.io/user-static/f64bc3cf-7421-4b53-9fbc-92c95ac672cb.glb"] = 1,
            ["https://hypar.io/user-static/fb5ca161-397a-41f3-a469-532b61a43405.glb"] = 1,

            ["https://hypar.io/user-static/6f807770-c291-4baf-9f5e-36cd3714270b.glb"] = 2,
            ["https://hypar.io/user-static/e21bbd93-8800-4632-a0c7-5084a903598b.glb"] = 2,
            ["https://hypar.io/user-static/12f48254-237b-493a-8e09-7c44d1102c23.glb"] = 2,
            ["https://hypar.io/user-static/4b4a1da6-2ceb-4a19-92d4-21209756fb5d.glb"] = 2,
            ["https://hypar.io/user-static/537d3aad-7cb1-4a4c-af8f-3889e9becce7.glb"] = 2,
            ["https://hypar.io/user-static/7809959f-ae7f-4ea6-aa1b-4a2b609a2ae3.glb"] = 2,
            ["https://hypar.io/user-static/b1e7fd86-3124-4f9d-9e0d-650f453df774.glb"] = 2,
            ["https://hypar.io/user-static/cdc18dac-396b-474d-a20b-2b1670a22a1a.glb"] = 2,
        };

LayoutFunctions/PrivateOfficeLayout/src/PrivateOfficeLayout.cs line 20 at r3 (raw file):

        private static readonly string SpaceBoundaryDependencyName = SpaceSettingsOverride.Dependency;

        private static Dictionary<string, int> _configSeats = new Dictionary<string, int>()

Can we add comments to here and also anywhere else you implemented _configSeats that this dictionary is a map between the layout and the number of seats it lays out?


WorkplaceStrategy/WorkplaceMetrics/dependencies/SpaceBoundary.cs line 10 at r3 (raw file):

    public partial class SpaceBoundary
    {
        public bool IsCounted { get; set; }

Can we be clear in comments about what IsCounted is supposed to mean? When would this be false? It looks like we check against both this and whether the name is circulation when we do the counting. It appears to be somewhat a proxy for whether we found a matching room for that space metric.

@KaterynaSloboda
Copy link
Contributor Author

Reviewed 2 of 37 files at r1, 9 of 16 files at r2, 17 of 17 files at r3, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained, 3 unresolved discussions (waiting on @KaterynaSloboda)

LayoutFunctions/LoungeLayout/src/LoungeLayout.cs line 59 at r3 (raw file):

            ["https://hypar.io/user-static/b1e7fd86-3124-4f9d-9e0d-650f453df774.glb"] = 2,
            ["https://hypar.io/user-static/cdc18dac-396b-474d-a20b-2b1670a22a1a.glb"] = 2,
        };

This does not appear to be used, is this for a future PR?

Code quote:

        private static Dictionary<string, int> _countableSeaters = new Dictionary<string, int>()
        {
            ["https://hypar.io/user-static/28ec56cc-3ae5-40df-a87f-9f116c1f9c62.glb"] = 1,
            ["https://hypar.io/user-static/2dcb44e3-71e5-4d50-93ba-c3abb259186f.glb"] = 1,
            ["https://hypar.io/user-static/31c06210-c409-4086-a496-1ec3093deddc.glb"] = 1,
            ["https://hypar.io/user-static/32534d22-4622-44ac-89d2-395f755ee4d9.glb"] = 1,
            ["https://hypar.io/user-static/47ba6d86-89ca-47b0-81e1-051919514ebd.glb"] = 1,
            ["https://hypar.io/user-static/48d2887c-543e-4b57-bc58-2cf4a132d5e7.glb"] = 1,
            ["https://hypar.io/user-static/75ccd3c4-4817-422f-86c5-f522ada7de37.glb"] = 1,
            ["https://hypar.io/user-static/8f5418c7-ad8b-474d-aa90-322f989f018c.glb"] = 1,
            ["https://hypar.io/user-static/93a62034-8f46-4312-a4be-bc043fe33206.glb"] = 1,
            ["https://hypar.io/user-static/a5c4260c-fd4e-4977-a4d0-a5f18ce283a2.glb"] = 1,
            ["https://hypar.io/user-static/b44b8d75-fc93-44e3-bca3-9c15502ffa25.glb"] = 1,
            ["https://hypar.io/user-static/c84d0092-962f-497e-ba1a-d5b25b61d1f4.glb"] = 1,
            ["https://hypar.io/user-static/d9dba349-e9e0-4d4e-a9b9-6e4652291f0c.glb"] = 1,
            ["https://hypar.io/user-static/f64bc3cf-7421-4b53-9fbc-92c95ac672cb.glb"] = 1,
            ["https://hypar.io/user-static/fb5ca161-397a-41f3-a469-532b61a43405.glb"] = 1,

            ["https://hypar.io/user-static/6f807770-c291-4baf-9f5e-36cd3714270b.glb"] = 2,
            ["https://hypar.io/user-static/e21bbd93-8800-4632-a0c7-5084a903598b.glb"] = 2,
            ["https://hypar.io/user-static/12f48254-237b-493a-8e09-7c44d1102c23.glb"] = 2,
            ["https://hypar.io/user-static/4b4a1da6-2ceb-4a19-92d4-21209756fb5d.glb"] = 2,
            ["https://hypar.io/user-static/537d3aad-7cb1-4a4c-af8f-3889e9becce7.glb"] = 2,
            ["https://hypar.io/user-static/7809959f-ae7f-4ea6-aa1b-4a2b609a2ae3.glb"] = 2,
            ["https://hypar.io/user-static/b1e7fd86-3124-4f9d-9e0d-650f453df774.glb"] = 2,
            ["https://hypar.io/user-static/cdc18dac-396b-474d-a20b-2b1670a22a1a.glb"] = 2,
        };

I'm sure we won't need it in the future. I used this to count the seats for each type of room. But after the task with the tests, I realized that it is easier to do this by visually evaluating not a separate element, but the room as a whole. Deleted it

LayoutFunctions/PrivateOfficeLayout/src/PrivateOfficeLayout.cs line 20 at r3 (raw file):

        private static readonly string SpaceBoundaryDependencyName = SpaceSettingsOverride.Dependency;

        private static Dictionary<string, int> _configSeats = new Dictionary<string, int>()

Can we add comments to here and also anywhere else you implemented _configSeats that this dictionary is a map between the layout and the number of seats it lays out?

Done

WorkplaceStrategy/WorkplaceMetrics/dependencies/SpaceBoundary.cs line 10 at r3 (raw file):

    public partial class SpaceBoundary
    {
        public bool IsCounted { get; set; }

Can we be clear in comments about what IsCounted is supposed to mean? When would this be false? It looks like we check against both this and whether the name is circulation when we do the counting. It appears to be somewhat a proxy for whether we found a matching room for that space metric.

I changed it to more understandable code. But if necessary, I can return the old one with comments

Copy link
Contributor

@serenayl serenayl left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 5 files at r4, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained, 2 unresolved discussions (waiting on @KaterynaSloboda)


LayoutFunctions/PrivateOfficeLayout/src/PrivateOfficeLayout.cs line 20 at r3 (raw file):

Previously, KaterynaSloboda wrote…

Done

Please modify this and the other places to use this format so that intellisense will pick it up:

        /// <summary>
        /// Map between the layout and the number of seats it lays out
        /// </summary>

Copy link
Contributor

@serenayl serenayl left a comment

Choose a reason for hiding this comment

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

Approving temporarily so that master is in sync.

Reviewable status: :shipit: complete! 1 of 1 approvals obtained, all discussions resolved

@serenayl serenayl merged commit 1ab7354 into hypar-io:master Jul 20, 2023
1 check passed
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

2 participants