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

feat: Add metadata fields to core entities #2861

Closed
wants to merge 42 commits into from

Conversation

novakzaballa
Copy link
Contributor

@novakzaballa novakzaballa commented Oct 18, 2023

Thanks for submitting a PR! Please check the boxes below:

  • I have run pre-commit to check linting
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

FE changes:

  • Add and create metadata to Environments, segments, and Flags
  • The feature was wrapped inside the 'enable_metadata' flag
    BE changes:
  • Support metadata for segments, and Flags
  • Support required metadata model fields for segments, and Flags

How did you test this code?

  • Go to manage
  • Click the Metadata tab.
  • Create Metadata.
  • Edit metadata and enable any entity

@vercel
Copy link

vercel bot commented Oct 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 21, 2023 6:05pm
flagsmith-frontend-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 21, 2023 6:05pm
flagsmith-frontend-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 21, 2023 6:05pm

@github-actions github-actions bot added the front-end Issue related to the React Front End Dashboard label Oct 18, 2023
@novakzaballa
Copy link
Contributor Author

I think this needs to go to design or at least more closely use the design system, e.g. making these column items checkboxes rather than the current ui and a less wordy edit cta.

I made some changes.

Do we explain what "Metadata" is where the user can create it ? Maybe a subtitle and a link to docs would be useful here.

I put a metadata subtitle, in the sections "managing features", "managing segments" and "environment settings" inside the docs.

title='Metadata'
isOpen={isOpen}
onClose={onToggle}
className='inline-modal--tags'
Copy link
Member

Choose a reason for hiding this comment

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

Can this be replaced with inline-modal--sm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

className='full-width mb-2'
placeholder='Type or choose a Metadata'
/>
<div style={{ maxHeight: 200, overflowY: 'auto' }}>
Copy link
Member

Choose a reason for hiding this comment

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

Can the inline style be replaced with className="inline-modal__list"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

placeholder='Type or choose a Metadata'
/>
<div style={{ maxHeight: 200, overflowY: 'auto' }}>
{metadataList &&
Copy link
Member

@kyle-ssg kyle-ssg Dec 18, 2023

Choose a reason for hiding this comment

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

What if there are none? Should display a default message

metadataList?.length? () : (<div className="text-center">No items</div>)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

space
>
<Flex
className={value?.includes(v.id) ? 'font-weight-bold' : ''}
Copy link
Member

Choose a reason for hiding this comment

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

If possible can we rename and reuse TableFilterItem.tsx? It does the exact same as this.

@@ -0,0 +1,37 @@
import { FC, useEffect, useState } from 'react'
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why it's called MyMetadata, that would indicate to me it's specific to the user but I think this is for the org?

<span className='chip-icon ion'>
<IonIcon
icon={closeIcon}
style={{ fontSize: '13px' }}
Copy link
Member

Choose a reason for hiding this comment

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

className="fs-caption"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

width: '185px',
}}
>
<div className='table-column'>
Copy link
Member

Choose a reason for hiding this comment

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

Markup here doesn't lookl correct, we are nesting table-column elements. Please lets use classnames where possible too.

                                    <div className='table-column d-flex text-center'>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}}
>
<div className='table-column'>
{metadata.content_type_fields.find(
Copy link
Member

Choose a reason for hiding this comment

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

Hard to read what the following functions are doing, perhaps separate these into variables above the markup ?

className='table-column'
style={{ width: '150px' }}
>
{metadata.content_type_fields.find(
Copy link
Member

Choose a reason for hiding this comment

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

Same here

}}
>
{metadata.content_type_fields.find(
(m) =>
Copy link
Member

Choose a reason for hiding this comment

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

And here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issue related to the REST API docs Documentation updates front-end Issue related to the React Front End Dashboard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add meta data fields to core entities
5 participants