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

[G2M] Bday modal #46

Merged
merged 5 commits into from
Nov 11, 2020
Merged

[G2M] Bday modal #46

merged 5 commits into from
Nov 11, 2020

Conversation

tlowande
Copy link
Contributor

@tlowande tlowande commented Oct 28, 2020

  • applies modal to sign and send options to account for multiple ppl
  • to be used in slack, need to do some bot integration and allow for interactivity
  • I'm open to rephrase things :)
  • not sure if it's ok to do the conditional response but it is needed for interactions with modal
  • if follows the order of:
    • if it is the first iteration > open modal

image

  • if more fields are required or removed

image

  • if modal has been submitted

image

but each of these if blocks have their own specificities.

sign sends a DM to everyone to sign. Looks like:
image

send looks like:
image
image
image

Comment on lines -100 to -113
// value standard: '[module] // param-1 // ...param-N'
const [module, taskId, customFieldId, value] = action_value.split(' // ')
if (module === 'vacay') {
updateTask(taskId, { custom_fields: { [customFieldId]: value }})
.then(res => {
const { assignee: { name }, start_on, due_on, custom_fields } = res
const status = custom_fields.find(o => o.name === 'Status').enum_value.name
const date = start_on ? `${start_on} - ${due_on}` : due_on
axios.post(response_url, {
text: `Updated *${name}'s* vacation on *${date}* to *${status}*`,
mrkdwn: true,
response_type: 'ephemeral',
replace_original: false,
})
Copy link
Contributor Author

@tlowande tlowande Oct 29, 2020

Choose a reason for hiding this comment

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

I didn't find any command that was using this block and the /vacay is not even working for me, lemme know if I missed something

Copy link
Contributor Author

@tlowande tlowande Oct 30, 2020

Choose a reason for hiding this comment

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

actually I just realized that there's a problem at worker. From lambda logs:

    "errorType": "TypeError",
    "errorMessage": "Cannot read property 'gid' of null",
    "stack": [
        "TypeError: Cannot read property 'gid' of null",
        "    at vacation.reduce.true (/var/task/modules/vacation.js:87:24)",
        "    at Array.reduce (<anonymous>)",
        "    at worker (/var/task/modules/vacation.js:84:16)",
        "    at async Runtime.module.exports.handler (/var/task/slack-worker.js:6:3)"
    ]
}

so I'm not sure if it was even using the interactive endpoint in the first place

@tlowande
Copy link
Contributor Author

tlowande commented Oct 29, 2020

I've been trying to use https://slack.dev/node-slack-sdk/interactive-messages middleware for /interactive, wondering if it would help with serverless issues. I also found this

when running apps on AWS Lambda (or any other FaaS), developers should be aware of the possibility of termination of app containers once apps return an HTTP response to Slack.

but I thought that by including the response_url or trigger_id in case of modals, the termination shouldn'd be an issue, am I missing something?

the npm package for interactive-messages can generate its own server and another option would be if we should host just the interactive part separately from lambda.

as a ref:

app.js Outdated Show resolved Hide resolved
app.js Outdated Show resolved Hide resolved
modules/bday.js Show resolved Hide resolved
modules/bday.js Outdated Show resolved Hide resolved
if (type === 'block_actions' && action.block_id === 'manage_fields') {
let updatedBlocks
let _buttons

Copy link
Contributor

Choose a reason for hiding this comment

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

seem like a repetitive code block except for a few params, maybe could extract as a common function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as it is a bit experimental, I'll leave it for now, but agree, could be transformed into a helper function.

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.

2 participants