Skip to content

Cla contributor table #277

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

nitinsubz
Copy link
Collaborator

This PR shows CLA's that a user is considered a contributor as.

At the bottom of the home CLA screen, there now exists a table that lists all the CLA's that the user is covered by.

A few questions:

  • Right now if there are no CLA's, then the table simply does not exist all together. Is that fine?
  • I am only getting the addendums for each agreement under 'AddendumType.CONTRIBUTOR'

Screen Shot 2021-08-23 at 11 25 20 PM

Fixes #196

@codecov
Copy link

codecov bot commented Aug 24, 2021

Codecov Report

Merging #277 (bea4351) into master (271efd6) will increase coverage by 0.72%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #277      +/-   ##
==========================================
+ Coverage   89.77%   90.50%   +0.72%     
==========================================
  Files           9        9              
  Lines         313      337      +24     
  Branches       22       25       +3     
==========================================
+ Hits          281      305      +24     
  Misses         32       32              
Impacted Files Coverage Δ
common/model/agreement.js 92.45% <100.00%> (+1.75%) ⬆️
common/model/appUser.js 96.77% <100.00%> (+0.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 271efd6...bea4351. Read the comment docs.

@teone
Copy link
Member

teone commented Sep 2, 2021

The feature appears to be working fine, but can we add the Signer of the Agreement and the Company (if any) to the table?

Screenshot from 2021-09-02 16-41-22

if (props.type === AgreementType.INDIVIDUAL) {
// remove the org name if we're printing individual CLAs
cols.shift()
// If there are extra columns, append them to the table, before the actions column
Copy link
Member

Choose a reason for hiding this comment

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

this seems a little convoluted, we can probably change the logic so that:

let base_cols = [...]
const action_cols = [...]

if (props.extra_cols && props.extra_cols.length > 0) {
  base_cols = [...base_cols, ...props.extra_cols] 
}

const cols = [...base_cols, ...action_cols]

In this way we don't have to shift arrays around

const cols = [
{ title: 'Organization', field: 'organization' },
let cols = [
// { title: 'Organization', field: 'organization' },
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this commented columns?

@@ -120,7 +119,7 @@ export default class Home extends React.Component {
<Grid item xs={12} md={12}>
<AgreementsTable
header='Covered by:'
type={AgreementType.INDIVIDUAL}
extra_cols={[{ title: 'Organization', field: 'organization' }, { title: 'Signer', field: 'signer.name'}]}
Copy link
Member

Choose a reason for hiding this comment

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

It appears that we already have a Signatory field, no need to duplicate it:

{
      title: 'Signatory',
      sorting: false,
      render: d => {
        return <div>{d.signer.name} <br/> <i>{d.signer.value}</i></div>
      },
      customFilterAndSearch: (query, data) => {
        return data.signer.name.indexOf(query) > -1 || data.signer.value.indexOf(query) > -1
      }
    },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Show list of CLA that are covering me as a contributor
2 participants