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/589 edit skillset #613

Merged
merged 51 commits into from
Mar 15, 2024
Merged

Feature/589 edit skillset #613

merged 51 commits into from
Mar 15, 2024

Conversation

Vakmeth
Copy link
Collaborator

@Vakmeth Vakmeth commented Feb 23, 2024

No description provided.

@Vakmeth Vakmeth self-assigned this Feb 23, 2024
@Vakmeth Vakmeth requested a review from kcinay055679 February 23, 2024 14:41
@Vakmeth Vakmeth force-pushed the feature/589-edit-skillset branch 2 times, most recently from b02b11d to 08c3ff9 Compare February 27, 2024 08:03
@Vakmeth Vakmeth marked this pull request as ready for review February 27, 2024 09:48
@Vakmeth Vakmeth requested a review from Robin481 February 27, 2024 10:03
Robin481
Robin481 requested changes Feb 27, 2024
View reviewed changes
Copy link
Member

@Robin481 Robin481 left a comment

Choose a reason for hiding this comment

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

Real nice functionality but I think we should cut back on the javascript used. See chat.

app/assets/stylesheets/bootstrap.scss Show resolved Hide resolved
app/assets/stylesheets/styles.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/styles.scss Outdated Show resolved Hide resolved
app/controllers/skills_controller.rb Outdated Show resolved Hide resolved
app/views/layouts/application.html.haml Outdated Show resolved Hide resolved
app/javascript/controllers/table_controller.js Outdated Show resolved Hide resolved
app/javascript/controllers/table_controller.js Outdated Show resolved Hide resolved
mtnstar
mtnstar suggested changes Mar 7, 2024
View reviewed changes
Copy link
Contributor

@mtnstar mtnstar left a comment

Choose a reason for hiding this comment

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

nice, you're getting there! 🥳

but still some things to make this bit more clean and pretty ...

go go go 🚀

app/assets/images/floppy2-fill.svg Outdated Show resolved Hide resolved
app/controllers/crud_controller.rb Outdated Show resolved Hide resolved
app/controllers/crud_controller.rb Show resolved Hide resolved
app/controllers/crud_controller.rb Outdated Show resolved Hide resolved
app/controllers/crud_controller.rb Outdated Show resolved Hide resolved
spec/controllers/skills_controller_spec.rb Outdated Show resolved Hide resolved
spec/domain/skills_filter_spec.rb Outdated Show resolved Hide resolved
spec/views/people/index_spec.rb Show resolved Hide resolved
app/javascript/controllers/index.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@mtnstar
Copy link
Contributor

mtnstar commented Mar 7, 2024

please also make sure we get an 404 when trying to access :edit/:update without turbo ...
respond_to in controller and/or format in routes

@Vakmeth Vakmeth requested a review from Robin481 March 8, 2024 14:46
@Vakmeth Vakmeth force-pushed the feature/589-edit-skillset branch from 9deda30 to 23a0ac6 Compare March 13, 2024 09:33
@Vakmeth Vakmeth assigned Robin481 and unassigned Vakmeth Mar 15, 2024
Vakmeth added 26 commits March 15, 2024 12:53
@Vakmeth
decrease default bootstrap padding of select
570e8c9
@Vakmeth
give style to other dropdown too
4056e8c
@Vakmeth
fix error of not defined option values
b67f20e
@Vakmeth
insert error tag below edit of row if form is invalid
62c9738
@Vakmeth
begin with testing
53142dc
@Vakmeth
finish test implementation
165ede3
@Vakmeth
increase width of content
4bb9455
@Vakmeth
fix tests after changing category of skill
9b1425c
@Vakmeth
change bootstrap classes on divs
60d758f
@Vakmeth
try using retry mechanism to fix test on pipeline
819ba9e
@Vakmeth
use sleep method to resolve timing problem
eac0eee
@Vakmeth
revert changes on yarn lock file
faadc47
@Vakmeth
replace sleep with capybara expectation
6166908
@Vakmeth
refactor edit functionality by reloading whole row after dropdown change
bed442b
@Vakmeth
use CRUD controller impelementation as approach instead of custom upd…
6d34ac7 
…ate action
@Vakmeth
fix error of wrong rerender by using requestSubmit method instead of …
be29c84 
…submit
@Vakmeth
fix text by adjusting name of query parameter and rework styles
f309af0
@Vakmeth
adjust ids of selects
69bf777
@Vakmeth
delete unnecessary within command in test
523acb0
@Vakmeth
rework tests by using different way to select option and adjusting ids
8b402ed
@Vakmeth
refactor code after pullrequest review
24d2de8
@Vakmeth
revert changes of capybara configuration
392b19d
@Vakmeth
resolve rubocop offenses
e4d498d
@Vakmeth
remove unnecessary method and fix error by assigning update variable …
93666d2 
…again
@Vakmeth
fix tests by including authorization step
31ea03f
@Vakmeth
resolve merge errors
7166d85
@Vakmeth Vakmeth force-pushed the feature/589-edit-skillset branch from 23a0ac6 to 7166d85 Compare March 15, 2024 12:41
@Robin481 Robin481 merged commit 2f03d3b into master Mar 15, 2024
2 checks passed
@Robin481 Robin481 deleted the feature/589-edit-skillset branch March 15, 2024 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtnstar mtnstar mtnstar requested changes

@Robin481 Robin481 Robin481 approved these changes

@kcinay055679 kcinay055679 Awaiting requested review from kcinay055679

Assignees

@Robin481 Robin481

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Vakmeth @mtnstar @Robin481 @kcinay055679