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.

Sign up for GitHub

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

Jump to bottom

feature/multi-element-action #467

Closed
wants to merge 35 commits into from
Closed

feature/multi-element-action #467

wants to merge 35 commits into from

Conversation

miltonbsn
Copy link
Member

@miltonbsn miltonbsn commented Sep 19, 2023 •
edited by idmarjr
Loading

Summary

Copy and paste allowed only on conceptual module.

Logic relations have deep internal connections (AKA foreign key), to copy and paste elements we have to update all these references and although not impossible, it is very complex and could lead to bugs.

In order to inform users about this limitation, the following message is now visible when users try to copy + paste elements in a logic model

image

Unrelated fixes

  • Fix email type on signup page

Close #456
Close #473

@idmarjr
Copy link
Collaborator

idmarjr commented Sep 19, 2023

Éx um monxtro!

I did some tests here and it's working smoothly.
Do you want to go above and beyond?

  1. Once only one element is selected, show the selection mark too (The one with green border)
  2. Allow user to select multiple elements one-by-one when holding cmd / ctrl and clicking in each element.

joaomfiumari and others added 18 commits September 24, 2023 16:30
@joaomfiumari @miltonbsn
feat(config): Add config file to centralize the env var loads
8b0e1aa
@joaomfiumari @miltonbsn
feat(auth): Add jwt middleware to validate received access-token
84551cb
@joaomfiumari @miltonbsn
feat(auth): Add interceptor to add jwt token in the requests
680f141
@dependabot
Bump jsonwebtoken from 8.5.1 to 9.0.0
e16750b 
Bumps [jsonwebtoken](https://github.com/auth0/node-jsonwebtoken) from 8.5.1 to 9.0.0.
- [Changelog](https://github.com/auth0/node-jsonwebtoken/blob/master/CHANGELOG.md)
- [Commits](auth0/node-jsonwebtoken@v8.5.1...v9.0.0)

---
updated-dependencies:
- dependency-name: jsonwebtoken
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
@miltonbsn
Merge pull request #468 from brmodeloweb/dependabot/npm_and_yarn/json…
1110104 
…webtoken-9.0.0

Bump jsonwebtoken from 8.5.1 to 9.0.0
@miltonbsn
refac: small improvement
02f38c6
@miltonbsn
refac: small improvement
3628981
@idmarjr
Update minor version devDependencies updates
d58b098
@idmarjr
Update Major devDependencies with 1x version
59b3d9c 
- Ex: v3.x.x -> v4.x.x
- Also, define type for children at Button.tsx to ANY
@idmarjr
Update rimraf: v3.x --> v5.x
0075b7f
@idmarjr
Update Cypress & Jest
fa5bdb5
@idmarjr
Merge pull request #471 from brmodeloweb/update-dev-dependencies
ee6129d
@idmarjr
Define missing children types
31c16b0 
Need after update @types to newer version
@idmarjr
Update react related dependencies
fe98e2e 
Update to match devDependencies versions and avoid breaking changes
As we currently have after I merge my last pull request ¯\_(ツ)_/¯
@idmarjr
Update dependencies with minor version changes
16c14fd
@idmarjr
Update dotenv dependency
eee8c0f
@idmarjr
Update i18next related dependencies
e2be9c2
@idmarjr
Merge pull request #472 from brmodeloweb/update-project-dependencies
e850dab
@idmarjr idmarjr mentioned this pull request Oct 4, 2023
Closed
@miltonbsn miltonbsn marked this pull request as ready for review October 8, 2023 13:03
@miltonbsn miltonbsn requested a review from idmarjr October 8, 2023 13:03
@miltonbsn
Copy link
Member Author

miltonbsn commented Oct 8, 2023 •
edited
Loading

Éx um monxtro!

I did some tests here and it's working smoothly. Do you want to go above and beyond?

  1. Once only one element is selected, show the selection mark too (The one with green border)
  2. Allow user to select multiple elements one-by-one when holding cmd / ctrl and clicking in each element.
  1. could not do it because the actions conflict
  2. I tried but did not work after spending good amount of time. Will try more but wont hold the PR

@idmarjr
Copy link
Collaborator

idmarjr commented Oct 8, 2023

@miltonbsn regarding the limitation to copy + paste in Logic models, I have the following question:

Considering Foreign key as the limitation factor, can we enable c+p if a table still don't have stablished relationships yet?
Meaning, should we enable user co c+p tables that dont have a connection with any other table?

idmarjr
idmarjr reviewed Oct 8, 2023
View reviewed changes
app/i18n/languages/en.js Outdated
Comment on lines 160 to 161
'Copy and paste is not allowed on this module due to complexity when updating references': 'Copy and paste is not allowed on this module due to complexity when updating references'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this copy being used?

Copy link
Member Author

@miltonbsn miltonbsn Oct 8, 2023 •
edited by idmarjr
Loading

Choose a reason for hiding this comment

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

Good catch! It was my first version but found too long, changed and forgot this here.

The new one will be:

EN: Copy is not allowed on this module when element has references.
PT-BR: Copiar não é permitido nesse módulo quando elementos já estiverem conectados.

What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good to me. Will you update it here or in a new branch?

Copy link
Member Author

Choose a reason for hiding this comment

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

Here, I am working on it right now. Let's try merge this today.

@idmarjr
Copy link
Collaborator

idmarjr commented Oct 8, 2023

On more thing...
Is this branch mixing two different fixes?

Branch name is feature/validate-mail and I also see some changes related to that... 😓

@miltonbsn
Copy link
Member Author

@miltonbsn regarding the limitation to copy + paste in Logic models, I have the following question:

Considering Foreign key as the limitation factor, can we enable c+p if a table still don't have stablished relationships yet? Meaning, should we enable user co c+p tables that dont have a connection with any other table?

Yesss, I was a bit lazy but will do it 😅

@miltonbsn
Copy link
Member Author

On more thing... Is this branch mixing two different fixes?

Branch name is feature/validate-mail and I also see some changes related to that... 😓

Yes, I started doing one thing and had this idea and delivered two. Sorry and forgive me this time 🙏

@idmarjr
Copy link
Collaborator

idmarjr commented Oct 8, 2023

Yes, I started doing one thing and had this idea and delivered two. Sorry and forgive me this time 🙏

judjing-you

@miltonbsn
Copy link
Member Author

miltonbsn commented Oct 8, 2023 •
edited
Loading

@idmarjr we are good to go again. I did a mistake when updating this branch but i will fix when merging this PR.

@miltonbsn miltonbsn requested a review from idmarjr October 8, 2023 20:28
@miltonbsn
Copy link
Member Author

@idmarjr wait, it is worst than I expected, i will open another PR

@miltonbsn miltonbsn closed this Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@idmarjr idmarjr Awaiting requested review from idmarjr

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deletar Poder deletar varios componentes apertando o botão [delete]
3 participants
@miltonbsn @idmarjr @joaomfiumari